bpo-36048: Use index() instead of int() for implicit conversion if available. #11952

serhiy-storchaka · 2019-02-20T06:54:22Z

Deprecate using the __int__() method in implicit conversions of Python numbers to C integers.

https://bugs.python.org/issue36048

…on if available. Deprecate using the __int__() method in implicit conversions of Python numbers to C integers.

vstinner

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?

Objects/longobject.c

Doc/c-api/long.rst

Lib/test/test_structmembers.py

serhiy-storchaka

Thank you for your review, @vstinner and @jdemeyer!

Lib/test/test_structmembers.py

Objects/longobject.c

mdickinson · 2019-02-20T18:46:20Z

@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.)

Objects/longobject.c

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.

cculianu · 2021-12-08T23:05:02Z

This is a terrible change. There is literally no sense in breaking floats used as parameters to C functions expecting int. Literally even the C or C++ language doesn't have this strict a type requirement. Like if I am in C and I have float foo = 42.42; and I call a function whose signature is: void bar(int); as: bar(foo); // <-- this works in C. So if that's ok in C, why make Python even more unergonomic than C here?

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.

arhadthedev · 2021-12-08T23:15:13Z

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.

@serhiy-storchaka It looks like this PR need to be retracted until PyQt5 is changed to newer Python C API.

cculianu · 2021-12-09T01:59:00Z

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 int

Same 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 QPoint takes 2 ints for its c'tor (just like in C++).. and breaks on Python 3.10 if given floats, when previously it worked just fine with either ints or floats. This is just 1 example. But many codebases that use PyQt5 are hit by breakages like this one now.

Good job guys! You just created a ton of busy work for people senselessly.

serhiy-storchaka · 2021-12-09T10:41:51Z

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.

arhadthedev · 2021-12-09T11:12:36Z

@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).

cculianu · 2021-12-09T16:54:59Z

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

cculianu · 2021-12-09T17:00:58Z

@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 float-like types -> int is well defined. Why restrict things? If user code gives you a float where an int is expected, that's well defined. Why restrict? It seems un-Pythonic to even restrict such a thing.. so I am very curious what the motivation is for this change.

mdickinson · 2021-12-09T17:59:16Z

@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.)

bpo-36048: Use __index__() instead of __int__() for implicit conversi…

9435aab

…on if available. Deprecate using the __int__() method in implicit conversions of Python numbers to C integers.

serhiy-storchaka added the type-feature A feature request or enhancement label Feb 20, 2019

the-knights-who-say-ni added the CLA signed label Feb 20, 2019

bedevere-bot added the awaiting merge label Feb 20, 2019

vstinner approved these changes Feb 20, 2019

View changes

Objects/longobject.c Outdated Show resolved Hide resolved

jdemeyer reviewed Feb 20, 2019

View changes

Doc/c-api/long.rst Outdated Show resolved Hide resolved

Doc/c-api/long.rst Outdated Show resolved Hide resolved

Lib/test/test_structmembers.py Outdated Show resolved Hide resolved

Address review comments.

6f2a051

serhiy-storchaka commented Feb 20, 2019

View changes

Lib/test/test_structmembers.py Outdated Show resolved Hide resolved

Objects/longobject.c Outdated Show resolved Hide resolved

mdickinson self-requested a review Feb 20, 2019

serhiy-storchaka commented Feb 20, 2019

View changes

Objects/longobject.c Outdated Show resolved Hide resolved

serhiy-storchaka added 3 commits Feb 22, 2019

Make PyNumber_Check() returning 1 for index integers.

b0aa18d

Use nb_index for the result of __trunc__.

44b5acd

Extend the warning message.

fbecff8

vstinner reviewed Feb 25, 2019

View changes

Objects/longobject.c Show resolved Hide resolved

serhiy-storchaka merged commit 6a44f6e into python:master Feb 25, 2019
5 checks passed

bedevere-bot removed the awaiting merge label Feb 25, 2019

serhiy-storchaka deleted the convert-python-number-to-c-int branch Feb 25, 2019

tacaswell mentioned this pull request Aug 29, 2019

regression in py38 with bool-as-index deprecation warning numpy/numpy#14397

Open

tacaswell mentioned this pull request Sep 1, 2019

MNT: explicitly cast np.bool_ -> bool to prevent deprecation warning matplotlib/matplotlib#15168

Merged

2 tasks

BlueMax mentioned this pull request Dec 13, 2021

Python3.10.1/PyQT - draw elements errors falkTX/Carla#1549

Closed

bpo-36048: Use index() instead of int() for implicit conversion if available. #11952

bpo-36048: Use index() instead of int() for implicit conversion if available. #11952

serhiy-storchaka commented Feb 20, 2019 •

edited

vstinner left a comment

serhiy-storchaka left a comment

mdickinson commented Feb 20, 2019

cculianu commented Dec 8, 2021 •

edited

arhadthedev commented Dec 8, 2021

cculianu commented Dec 9, 2021 •

edited

serhiy-storchaka commented Dec 9, 2021

arhadthedev commented Dec 9, 2021

cculianu commented Dec 9, 2021

cculianu commented Dec 9, 2021 •

edited

mdickinson commented Dec 9, 2021 •

edited

bpo-36048: Use __index__() instead of __int__() for implicit conversion if available. #11952

bpo-36048: Use __index__() instead of __int__() for implicit conversion if available. #11952

Conversation

serhiy-storchaka commented Feb 20, 2019 • edited

vstinner left a comment

serhiy-storchaka left a comment

mdickinson commented Feb 20, 2019

cculianu commented Dec 8, 2021 • edited

arhadthedev commented Dec 8, 2021

cculianu commented Dec 9, 2021 • edited

serhiy-storchaka commented Dec 9, 2021

arhadthedev commented Dec 9, 2021

cculianu commented Dec 9, 2021

cculianu commented Dec 9, 2021 • edited

mdickinson commented Dec 9, 2021 • edited

bpo-36048: Use index() instead of int() for implicit conversion if available. #11952

bpo-36048: Use index() instead of int() for implicit conversion if available. #11952

serhiy-storchaka commented Feb 20, 2019 •

edited

cculianu commented Dec 8, 2021 •

edited

cculianu commented Dec 9, 2021 •

edited

cculianu commented Dec 9, 2021 •

edited

mdickinson commented Dec 9, 2021 •

edited