bpo-26680: Incorporate is_integer in all built-in and standard library numeric types#6121
bpo-26680: Incorporate is_integer in all built-in and standard library numeric types#6121mdickinson merged 10 commits into
Conversation
|
This issue is probably languishing because of the amount of controversy it generated. Was there a pronouncement from Guido on the mailing lists? Regardless, it would be nice to get at least +-0 from @mdickinson @rhettinger @tim-one. I'm +1 on this. |
Sorry, something went wrong.
|
@skrah Yes. Guido gave a pronouncement on python-dev. |
Sorry, something went wrong.
+1 in principle. I haven't reviewed the code changes, and am unlikely to have the bandwidth to do so in the near future. |
Sorry, something went wrong.
|
@skrah Is there anything I can do to help move this along? |
Sorry, something went wrong.
|
@rob-smallshire This is basically just waiting for review now that @mdickinson is also in favor. I probably have time to review this before the beta-1 release, which is 2019-05-27. |
Sorry, something went wrong.
mdickinson
left a comment
There was a problem hiding this comment.
Mostly LGTM; a few comments and suggestions.
We'll need a "whatsnew" entry, and we're missing some ".. versionchanged: " notes in the documentation updates.
Sorry, something went wrong.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
Sorry, something went wrong.
|
I haven't had capacity to do the requisite changes for 3.8. I propose going for 3.9. |
Sorry, something went wrong.
|
@rob-smallshire, please address @mdickinson's review comments. Once everything looks good, it will make it into whichever release is master at the time, which right now would be 3.10. Thanks! |
Sorry, something went wrong.
|
@csabella Will do. I've scheduled some time to dedicate to this in early June. |
Sorry, something went wrong.
|
@eric-wieser Yes, absolutely. I've been looking at the required changes. I'll be pushing updates soon. |
Sorry, something went wrong.
e745028 to
e6a66a7
Compare
September 17, 2020 11:54
|
Status update: rebased to resume work on the outstanding issues. |
Sorry, something went wrong.
e6a66a7 to
1cacea7
Compare
September 17, 2020 12:03
Thanks! Please ping me when you're done with updates and ready for re-review. |
Sorry, something went wrong.
…loat.is_integer(). The int.is_integer() method always returns True.
…integer() are always True.
1cacea7 to
138f704
Compare
September 18, 2020 09:16
138f704 to
a6a5490
Compare
September 18, 2020 10:03
… conversion to int. This default implementation is intended to reduce the workload for subclass implementers. It is not robust in the presence of infinities or NaNs and may have suboptimal performance for other types.
…ator is one. This implementation assumes the Rational is represented in it's lowest form, as required by the class docstring.
1ded358 to
821aad6
Compare
September 18, 2020 13:00
…t and float. The call x.is_integer() is now listed in the table of operations which apply to all numeric types except complex, with a reference to the full documentation for Real.is_integer(). Mention of is_integer() has been removed from the section 'Additional Methods on Float'. The documentation for Real.is_integer() describes its purpose, and mentions that it should be overridden for performance reasons, or to handle special values like NaN.
821aad6 to
fba90b3
Compare
September 18, 2020 14:32
The C implementation of Decimal already implements and uses mpd_isinteger internally, we just expose the existing function to Python. The Python implementation uses internal conversion to integer using to_integral_value(). In both cases, the corresponding context methods are also implemented. Tests and documentation are included.
fba90b3 to
11db7f8
Compare
September 18, 2020 14:52
|
@mdickinson I believe I now have made all the changes you requested, and all checks are passing. |
Sorry, something went wrong.
mdickinson
left a comment
There was a problem hiding this comment.
LGTM. All my comments are addressed.
Sorry, something went wrong.
|
I'll leave this open for a couple of days in case @rhettinger or @skrah has further comments. |
Sorry, something went wrong.
|
Anything I can do to help get this merged @mdickinson ? |
Sorry, something went wrong.
Apart from pinging me in case I've forgotten? Nothing else I can think of ... Merging shortly. Thank you! |
Sorry, something went wrong.
⚠️⚠️⚠️ Buildbot failure ⚠️⚠️⚠️Hi! The buildbot aarch64 RHEL7 3.x has failed when building commit 58a7da9. What do you need to do:
You can take a look at the buildbot page here: https://buildbot.python.org/all/#builders/539/builds/133 Summary of the results of the build (if available): == Tests result: ENV CHANGED == 408 tests OK. 10 slowest tests:
1 test altered the execution environment: 14 tests skipped: Total duration: 8 min 3 sec Click to see traceback logsTraceback (most recent call last):
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/sslproto.py", line 321, in __del__
self.close()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/sslproto.py", line 316, in close
self._ssl_protocol._start_shutdown()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/sslproto.py", line 590, in _start_shutdown
self._abort()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/sslproto.py", line 731, in _abort
self._transport.abort()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/selector_events.py", line 680, in abort
self._force_close(None)
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/selector_events.py", line 731, in _force_close
self._loop.call_soon(self._call_connection_lost, exc)
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/base_events.py", line 746, in call_soon
self._check_closed()
File "/home/buildbot/buildarea/3.x.cstratak-RHEL7-aarch64/build/Lib/asyncio/base_events.py", line 510, in _check_closed
raise RuntimeError('Event loop is closed')
RuntimeError: Event loop is closed
|
Sorry, something went wrong.
|
It seems rather unlikely that that buildbot failure is related (test_asyncio changed the environment). But it's odd that I don't see the same buildbot failure on other recent PRs. |
Sorry, something went wrong.
|
Why was this merged? Guido was clear, "It should not go on the numeric tower." |
Sorry, something went wrong.
* origin/master: (147 commits) Fix the attribute names in the docstring of GenericAlias (pythonGH-22594) bpo-39337: Add a test case for normalizing of codec names (pythonGH-19069) bpo-41557: Update Windows installer to use SQLite 3.33.0 (pythonGH-21960) bpo-41976: Fix the fallback to gcc of ctypes.util.find_library when using gcc>9 (pythonGH-22598) bpo-41306: Allow scale value to not be rounded (pythonGH-21715) bpo-41970: Avoid test failure in test_lib2to3 if the module is already imported (pythonGH-22595) bpo-41376: Fix the documentation of `site.getusersitepackages()` (pythonGH-21602) Revert "bpo-26680: Incorporate is_integer in all built-in and standard library numeric types (pythonGH-6121)" (pythonGH-22584) bpo-41923: PEP 613: Add TypeAlias to typing module (python#22532) Fix comment about PyObject_IsTrue. (pythonGH-22343) bpo-38605: Make 'from __future__ import annotations' the default (pythonGH-20434) bpo-41905: Add abc.update_abstractmethods() (pythonGH-22485) bpo-41944: No longer call eval() on content received via HTTP in the UnicodeNames tests (pythonGH-22575) bpo-41944: No longer call eval() on content received via HTTP in the CJK codec tests (pythonGH-22566) Post 3.10.0a1 Python 3.10.0a1 bpo-41584: clarify when the reflected method of a binary arithemtic operator is called (python#22505) bpo-41939: Fix test_site.test_license_exists_at_url() (python#22559) bpo-41774: Tweak new programming FAQ entry (pythonGH-22562) bpo-41936. Remove macros Py_ALLOW_RECURSION/Py_END_ALLOW_RECURSION (pythonGH-22552) ...
…y numeric types (pythonGH-6121) * bpo-26680: Adds support for int.is_integer() for compatibility with float.is_integer(). The int.is_integer() method always returns True. * bpo-26680: Adds a test to ensure that False.is_integer() and True.is_integer() are always True. * bpo-26680: Adds Real.is_integer() with a trivial implementation using conversion to int. This default implementation is intended to reduce the workload for subclass implementers. It is not robust in the presence of infinities or NaNs and may have suboptimal performance for other types. * bpo-26680: Adds Rational.is_integer which returns True if the denominator is one. This implementation assumes the Rational is represented in it's lowest form, as required by the class docstring. * bpo-26680: Adds Integral.is_integer which always returns True. * bpo-26680: Adds tests for Fraction.is_integer called as an instance method. The tests for the Rational abstract base class use an unbound method to sidestep the inability to directly instantiate Rational. These tests check that everything works correct as an instance method. * bpo-26680: Updates documentation for Real.is_integer and built-ins int and float. The call x.is_integer() is now listed in the table of operations which apply to all numeric types except complex, with a reference to the full documentation for Real.is_integer(). Mention of is_integer() has been removed from the section 'Additional Methods on Float'. The documentation for Real.is_integer() describes its purpose, and mentions that it should be overridden for performance reasons, or to handle special values like NaN. * bpo-26680: Adds Decimal.is_integer to the Python and C implementations. The C implementation of Decimal already implements and uses mpd_isinteger internally, we just expose the existing function to Python. The Python implementation uses internal conversion to integer using to_integral_value(). In both cases, the corresponding context methods are also implemented. Tests and documentation are included. * bpo-26680: Updates the ACKS file. * bpo-26680: NEWS entries for int, the numeric ABCs and Decimal. Co-authored-by: Robert Smallshire <rob@sixty-north.com>
…d library numeric types (pythonGH-6121)" (pythonGH-22584) This reverts commit 58a7da9.
Match `int.is_integer`, which was added in python/cpython#6121
Match `int.is_integer`, which was added in python/cpython#6121
This change implements support for the
x.is_integer()predicate across all built-in and standard library concrete numeric types:int,bool,FractionandDecimal; previously it was supported only onfloat. It also incorporatesis_integer()into the abstract typesReal,RationalandIntegral, with appropriate default implementations at each level.Updates to the relevant documentation and test suites are included.
https://bugs.python.org/issue26680