Do not fallback to __trunc__ when convert to int

Currently __trunc__ is used for two purposes: * In math.trunc() which just calls __trunc__. * In int() and PyNumber_Long() as a fallback if neither __int__ nor __index__ are implemented. Unlike to __int__ and __index__ there is not a slot corresponding to __trunc__. So using it is slower than using __int__ or __index__. float and Decimal implement __int__ and __trunc__ by the same code. Only Fraction implements __trunc__ but not __int__. I propose to deprecate the falling back to __trunc__ for converting to int and remove it in future. Fraction should implement __int__. We cannot use __trunc__ for setting the nb_int slot because it can return non-int.

It seems a good idea to add __int__ to Fraction, but if you stop falling back to __trunc__, won't that cause backwards compatibility issues? I'd say looking for __trunc__ if the alternative is failing isn't so bad. On Thu, Oct 31, 2019 at 3:43 AM Serhiy Storchaka <storchaka@gmail.com> wrote:
-- --Guido van Rossum (python.org/~guido) *Pronouns: he/him **(why is my pronoun here?)* <http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-c...>

On Fri., 1 Nov. 2019, 8:10 am Guido van Rossum, <guido@python.org> wrote:
Aye, converting a case where code is slower than it needs to be to an outright failure isn't a good trade-off. "Implements __trunc__ but not __int__" could be a good thing for linters to warn about, though. Cheers, Nick.

On Sat, Nov 2, 2019 at 1:54 PM Nick Coghlan <ncoghlan@gmail.com> wrote:
I find int() falling back to __trunc__() surprising, actually. Removing it will break backwards-compatibility, but implementing __int__() is an easy fix. Perhaps we could run a code search for classes implementing __trunc__ but not __int__? Speeding up int(str), as well as slightly simplifying the language, are rather good arguments in favor of this. - Tal

On Sun., 3 Nov. 2019, 3:29 am Tal Einat, <taleinat@gmail.com> wrote:
This is exactly the kind of change that prompted Victor to write PEP 608: seeing a compatibility break as our first option rather than as a last resort. Each one is cheap on its own, but collectively they make supporting new CPython versions more painful than it really needs to be.
In this specific case, they're not particularly strong arguments. * For performance, there shouldn't be any problem with making the check for __trunc__ the very last conversion tried. That will still speed up all successful int conversions that currently incur the cost of checking for a non-existent __trunc__ method, and could only cause a compatibility issue for obscure cases where a type that defines __trunc__, but not __int__, gives a different answer when handled via one of the type-specific conversions instead. * For simplification, "versions up to X allow A, versions after X only allow B" is never actually simpler, it's always more complex than just leaving things alone. It's just that the cost is sometimes worth it when the status quo is causing actual code correctness bugs. However, what could genuinely be simpler in this case (as discussed in https://bugs.python.org/issue33039 and https://bugs.python.org/issue20092) is to move the fallback logic to type creation time rather than requiring that it be implemented in every function that accepts Integral types as input. That logic would be: * if __index__ exists, make __int__ and __trunc__ wrappers around that if they don't already exist * if __int__ exists, but not __trunc__ or __index__, make __trunc__ a wrapper around __int__ * if __trunc__ exists, but not __int__ or __index__, make __int__ a wrapper around __trunc__ With that logic in place, consumers could just call whichever API made the most sense for their use case, and trust the type machinery to provide any appropriate fallbacks. Cheers, Nick.
- Tal

01.11.19 00:03, Guido van Rossum пише:
It seems a good idea to add __int__ to Fraction, but if you stop falling back to __trunc__, won't that cause backwards compatibility issues?
Most numeric types (int, float, Decimal, NumPy types) implement both __trunc__ and __int__. The only known exception is Fraction. So after adding __int__ to Fraction and with a deprecation period the possibility of backwards compatibility issues is pretty low.
I'd say looking for __trunc__ if the alternative is failing isn't so bad.
__trunc__ is looked after __int__ and __index__. But it is looked before type checks for str, bytes, bytearray and buffer protocol. This slows down integer parsing. Getting rid of __trunc__ could speed up int('123'). Other possible benefit is that it would allow to check ahead whether the object can be converted to int. Checking tp_as_number->nb_int and tp_as_number->nb_index is cheaper than looking up __trunc__ in the type dict and never executes Python code.

Le jeu. 31 oct. 2019 à 11:41, Serhiy Storchaka <storchaka@gmail.com> a écrit :
I propose to deprecate the falling back to __trunc__ for converting to int and remove it in future.
As Nick already said, I'm working on PEP 608 "Coordinated Python release" which discuss such change: https://www.python.org/dev/peps/pep-0608/ It's discussed at: https://discuss.python.org/t/rfc-pep-608-coordinated-python-release/2539 Do you have an idea of how many projects would emit such warning? More and more projects are running their test suite using -Werror. Any new warning can make a project test suite to fail. I'm looking for help to build a tool to test popular PyPI projects on a modified Python version, to be able to answer to such questions. My current experimental project can only test numpy, coverage and jinja2, and it doesn't work well (especially on Python 3.9): https://github.com/vstinner/pythonci/ Since this tool doesn't exist yet and since we cannot test all PyPI projects, I suggest to keep the option of reverting such change if we consider that it breaks too many projects during the Python 3.9 beta phase. Victor -- Night gathers, and now my watch begins. It shall not end until my death.

I don't have an opinion on __trunc__, but as someone who runs such a project, I have to say this is exactly the point. I always have the option of adding a given warning to my ignore list (as long as it's raised in such a way that this is easy to do), and I want the hard error so I can heed the warnings. Avoiding raising warnings because too many test suites break when using -Werror seems counterproductive. Of course, "lots of stuff breaks with -Werror" could be a proxy for "it would be a lot of work to fix this", and maybe all that work isn't worth it for whatever improvement you'd get from the fixes, but I think it would be better to try and evaluate it on those terms rather than considering broken test suites an inherently bad thing. On November 5, 2019 1:06:42 AM UTC, Victor Stinner <vstinner@python.org> wrote:

It seems a good idea to add __int__ to Fraction, but if you stop falling back to __trunc__, won't that cause backwards compatibility issues? I'd say looking for __trunc__ if the alternative is failing isn't so bad. On Thu, Oct 31, 2019 at 3:43 AM Serhiy Storchaka <storchaka@gmail.com> wrote:
-- --Guido van Rossum (python.org/~guido) *Pronouns: he/him **(why is my pronoun here?)* <http://feministing.com/2015/02/03/how-using-they-as-a-singular-pronoun-can-c...>

On Fri., 1 Nov. 2019, 8:10 am Guido van Rossum, <guido@python.org> wrote:
Aye, converting a case where code is slower than it needs to be to an outright failure isn't a good trade-off. "Implements __trunc__ but not __int__" could be a good thing for linters to warn about, though. Cheers, Nick.

On Sat, Nov 2, 2019 at 1:54 PM Nick Coghlan <ncoghlan@gmail.com> wrote:
I find int() falling back to __trunc__() surprising, actually. Removing it will break backwards-compatibility, but implementing __int__() is an easy fix. Perhaps we could run a code search for classes implementing __trunc__ but not __int__? Speeding up int(str), as well as slightly simplifying the language, are rather good arguments in favor of this. - Tal

On Sun., 3 Nov. 2019, 3:29 am Tal Einat, <taleinat@gmail.com> wrote:
This is exactly the kind of change that prompted Victor to write PEP 608: seeing a compatibility break as our first option rather than as a last resort. Each one is cheap on its own, but collectively they make supporting new CPython versions more painful than it really needs to be.
In this specific case, they're not particularly strong arguments. * For performance, there shouldn't be any problem with making the check for __trunc__ the very last conversion tried. That will still speed up all successful int conversions that currently incur the cost of checking for a non-existent __trunc__ method, and could only cause a compatibility issue for obscure cases where a type that defines __trunc__, but not __int__, gives a different answer when handled via one of the type-specific conversions instead. * For simplification, "versions up to X allow A, versions after X only allow B" is never actually simpler, it's always more complex than just leaving things alone. It's just that the cost is sometimes worth it when the status quo is causing actual code correctness bugs. However, what could genuinely be simpler in this case (as discussed in https://bugs.python.org/issue33039 and https://bugs.python.org/issue20092) is to move the fallback logic to type creation time rather than requiring that it be implemented in every function that accepts Integral types as input. That logic would be: * if __index__ exists, make __int__ and __trunc__ wrappers around that if they don't already exist * if __int__ exists, but not __trunc__ or __index__, make __trunc__ a wrapper around __int__ * if __trunc__ exists, but not __int__ or __index__, make __int__ a wrapper around __trunc__ With that logic in place, consumers could just call whichever API made the most sense for their use case, and trust the type machinery to provide any appropriate fallbacks. Cheers, Nick.
- Tal

01.11.19 00:03, Guido van Rossum пише:
It seems a good idea to add __int__ to Fraction, but if you stop falling back to __trunc__, won't that cause backwards compatibility issues?
Most numeric types (int, float, Decimal, NumPy types) implement both __trunc__ and __int__. The only known exception is Fraction. So after adding __int__ to Fraction and with a deprecation period the possibility of backwards compatibility issues is pretty low.
I'd say looking for __trunc__ if the alternative is failing isn't so bad.
__trunc__ is looked after __int__ and __index__. But it is looked before type checks for str, bytes, bytearray and buffer protocol. This slows down integer parsing. Getting rid of __trunc__ could speed up int('123'). Other possible benefit is that it would allow to check ahead whether the object can be converted to int. Checking tp_as_number->nb_int and tp_as_number->nb_index is cheaper than looking up __trunc__ in the type dict and never executes Python code.

Le jeu. 31 oct. 2019 à 11:41, Serhiy Storchaka <storchaka@gmail.com> a écrit :
I propose to deprecate the falling back to __trunc__ for converting to int and remove it in future.
As Nick already said, I'm working on PEP 608 "Coordinated Python release" which discuss such change: https://www.python.org/dev/peps/pep-0608/ It's discussed at: https://discuss.python.org/t/rfc-pep-608-coordinated-python-release/2539 Do you have an idea of how many projects would emit such warning? More and more projects are running their test suite using -Werror. Any new warning can make a project test suite to fail. I'm looking for help to build a tool to test popular PyPI projects on a modified Python version, to be able to answer to such questions. My current experimental project can only test numpy, coverage and jinja2, and it doesn't work well (especially on Python 3.9): https://github.com/vstinner/pythonci/ Since this tool doesn't exist yet and since we cannot test all PyPI projects, I suggest to keep the option of reverting such change if we consider that it breaks too many projects during the Python 3.9 beta phase. Victor -- Night gathers, and now my watch begins. It shall not end until my death.

I don't have an opinion on __trunc__, but as someone who runs such a project, I have to say this is exactly the point. I always have the option of adding a given warning to my ignore list (as long as it's raised in such a way that this is easy to do), and I want the hard error so I can heed the warnings. Avoiding raising warnings because too many test suites break when using -Werror seems counterproductive. Of course, "lots of stuff breaks with -Werror" could be a proxy for "it would be a lot of work to fix this", and maybe all that work isn't worth it for whatever improvement you'd get from the fixes, but I think it would be better to try and evaluate it on those terms rather than considering broken test suites an inherently bad thing. On November 5, 2019 1:06:42 AM UTC, Victor Stinner <vstinner@python.org> wrote:
participants (6)
-
Guido van Rossum
-
Nick Coghlan
-
Paul G
-
Serhiy Storchaka
-
Tal Einat
-
Victor Stinner