bpo-36048: Use __index__() instead of __int__() for implicit conversion if available. #11952
Conversation
…on if available. Deprecate using the __int__() method in implicit conversions of Python numbers to C integers.
There was a problem hiding this comment.
LGTM, but I would prefer that at least another core dev review the change. I quickly read the PR. I like the overall idea.
@mdickinson: Would you mind to review this change?
I'm a big +1 on the overall idea; we should have done this long ago. I'm probably not going to have the bandwidth to do a thorough review before the weekend. (But I trust @serhiy-storchaka to get the details right.) |
This is a workaround to https://bugs.python.org/issue37980 - np.bool raises a warning if you try to use it as an index (by warning in its `__index__` method - in py38 python/cpython#11952 python changes the code path used to convert `np.bool_` -> int for as it is used in `sorted` so it now goes through the `__index__` code path - this causes a bunch of spurious warnings to come out of Matplotlib.
This is a workaround to https://bugs.python.org/issue37980 - np.bool raises a warning if you try to use it as an index (by warning in its `__index__` method - in py38 python/cpython#11952 python changes the code path used to convert `np.bool_` -> int for as it is used in `sorted` so it now goes through the `__index__` code path - this causes a bunch of spurious warnings to come out of Matplotlib.
This is a workaround to https://bugs.python.org/issue37980 - np.bool raises a warning if you try to use it as an index (by warning in its `__index__` method - in py38 python/cpython#11952 python changes the code path used to convert `np.bool_` -> int for as it is used in `sorted` so it now goes through the `__index__` code path - this causes a bunch of spurious warnings to come out of Matplotlib.
|
This is a terrible change. There is literally no sense in breaking Why do I care? This breaks tons of existing PyQt5 code out there, for example. I wasn't aware of this change to the language until Fedora shipped Python 3.10 and everything broke. So much stuff that uses PyQt5 is broken now. Good job guys!! You guys just made everybody's life worse on the planet, with little in the way of reasoned, researched justification for it. Just design by committee fiat/capriciousness. This is bad design. Take it from me, I've been writing software for 20+ years. You just made the language worse for everybody, especially with 3.10 where this is now enforced strictly, with no way to turn it off. Bad bad bad. |
@serhiy-storchaka It looks like this PR need to be retracted until PyQt5 is changed to newer Python C API. |
|
Literally this is ok in C++ with Qt: float x = 2.3, y = 1.1;
auto p = QPoint(x, y); // QPoint only takes 2 int params.. this works in C++; floats can be always be implicitly converted to intSame code, more or less, in Python3.10 is now broken: x = 2.3
y = 1.1
p = QPoint(x, y) # This fails, where previously it worked on every Python version since the age of the dinosaurs...Note that most of the API for PyQt5 is auto-generated from the function signatures of the C++. So in this case Good job guys! You just created a ton of busy work for people senselessly. |
|
It is too late. This PR was merged more than 2.5 years ago. The changes was released in Python 3.8, which only accept security fixes now. If you want to propose some changes please open a new issue on the bug tracker or a thread on a mailing list for new discussion. A long time ago closed PR is not suitable place for this. |
|
@cculianu It is rather strange that maintainers of Fedora let coexist incompatible versions of Python and PyQt in their package repository. Probably you need to report them to either upgrade PyQt or downgrade Python. For PyQt, a site of its developer is rather obscure so I could not find source code for PyQt5 (I found PyQt4 only) to check if there is a Python 10 aware version. I recommend to ask them in their mailing list. Note: as for this (Python) PR, the bug tracker entry is https://bugs.python.org/issue36048 (as referenced in the first message of the PR). All discussions are there (indeed, I reposted your report into that thread). |
|
Heh, "even though the rest of the comment is emotional" ha ha you got me! Ok well thanks for escalating it and/or seeing what's going on. So -- I am not familiar with the internal machinery for the C/Python API layer. Are you saying this problem should largely be invisible to a properly maintained library that has such an interface layer -- and it may only be isolated to PyQt5 ? Anyway, I'll follow the discussion on the bugs.python.org thread. Thanks @arhadthedev |
|
@arhadthedev Oleg, I am having a hard time navigating the python bug tracker. Is there any place I can find a motivation or rationale for this change to Python? I see vague allusions to "We have seen problems over the years from Decimal being converted to int", but nothing specific. What problems? My feeling is that this change doesn't make sense for a language such as Python. Clearly mathematically, and in computer science, a conversion from |
|
@cculianu A closed, two-year-old PR isn't really the right place for this discussion, not least because most interested parties won't notice it. For better visibility and more responses, you might try discuss.python.org or the python-list mailing list. (Or something more generic, like Stack Overflow.) |
…ted as integers Prior to pythongh-11952, several standard library functions that expected integer arguments would nevertheless silently accept (and truncate) non-integer arguments. This behaviour was deprecated in pythongh-11952, and removed in pythongh-15636. However, it may be possible to interpret some non-integer numeric types (such as `decimal.Decimal`s) as integers if they contain no fractional part. Implement `__index__` for `decimal.Decimal`, returning an integer representation of the value if it does not contain a fractional part or raising a `TypeError` if it does.
Deprecate using the
__int__()method in implicit conversions of Python numbers to C integers.https://bugs.python.org/issue36048