Skip to content

Interpret decimal.Decimals as integers if no precision would be lost #102791

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
chrisnovakovic opened this issue Mar 17, 2023 · 7 comments
Closed
Labels
type-feature A feature request or enhancement

Comments

@chrisnovakovic
Copy link

chrisnovakovic commented Mar 17, 2023

Feature or enhancement

It should be possible for decimal.Decimals (and potentially other non-integer numeric types) to be interpreted as integers if doing so would not cause a loss of precision.

Pitch

As of GH-15636 (part of Python from 3.10.0 onwards), it's no longer possible to use certain numeric types as arguments to standard library functions when an integer is expected due to the removal of type coercion logic. This is, of course, quite sensible: datetime.datetime(year=2023, month=3, day=17.5) is meaningless, and should result in a TypeError rather than a datetime object representing 2023-03-17 00:00:00. However, there are situations where certain numeric types might be coercible to integers - one example is a decimal.Decimal with no fractional part. In these situations, I think it makes sense for the standard library functions whose behaviour changed following GH-15636 to behave as they did before.

In the Decimal case, this would involve implementing __index__ in a way that returns an integer representation of the value if the number has no fractional part, or raising a TypeError if it does. I believe this complies with the general understanding of how __index__ is supposed to work - from the What's New entry discussing PEP 357:

The return value must be either a Python integer or long integer. The interpreter will check that the type returned is correct, and raises a TypeError if this requirement isn't met.

I've implemented this in both _decimal and _pydecimal, with the following results:

>>> from decimal import Decimal
>>> from datetime import datetime
>>> datetime(year=Decimal(2023), month=Decimal(3), day=Decimal(17))
datetime.datetime(2023, 3, 17, 0, 0)
>>> datetime(year=Decimal(2023), month=Decimal(3), day=Decimal(17.5))
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: cannot convert Decimal with fractional part to integer

I have a particular need for this to work with Decimal, but perhaps the same could be done for other types in the standard library if there's enough interest.

Previous discussion

In issue36048, Serhiy discusses the rationale for this behaviour being deprecated in the first place (in Python 3.8) - the comment identifies the problem with "dropping the fractional part if the object is not [an] integral number", but doesn't address the corollary of that (i.e. permitting type coercion if the object is an integral number), which is the purpose of this issue.

Linked PRs

@chrisnovakovic chrisnovakovic added the type-feature A feature request or enhancement label Mar 17, 2023
chrisnovakovic added a commit to chrisnovakovic/cpython that referenced this issue Mar 17, 2023
…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.
@stevendaprano
Copy link
Member

I have a particular need for this to work with Decimal

Care to elaborate why you need this, or is it a secret? 😉

(For brevity in typing, I'm going to use float rather than Decimal.)

In the absence of any obvious or clear reason for this, it seems to me that this would just lay hidden land-mines in peoples code. E.g. they do something like datetime(year=2023.0, month=3.0, day=17.0) and see that it works, and then one day they end up using a computed value like 10.999999999999998 instead of their expected 11.0 and they get an unexpected TypeError.

@rhettinger
Copy link
Contributor

I think this idea goes against the spirit of what __index__ was designed to accomplish. For example, it is intentional that hex(100.0) raises a TypeError.

Also, this would make it harder for static type checkers to validate that code is correct.

And, as Steven points out, types with fractional components tend to had round-off errors that would make this a fragile and error-prone feature.

@ericvsmith
Copy link
Member

I agree it would be fragile and a bad idea to have __index__ fail or succeed based on the value. Type checkers could no longer catch this error. Better to be explicit in your code.

@sobolevn
Copy link
Member

Also, this would make it harder for static type checkers to validate that code is correct.

Right now typeshed has int annotations for all the fields: https://github.com/python/typeshed/blob/ca55889329ccdf05be8772496ba937aafc4aebbb/stdlib/datetime.pyi#L224-L233

Basically, we would need to use SupportsIndex protocol in a lot of places to support Decimal(1)

@mdickinson
Copy link
Member

Agreed with other commenters that Decimal shouldn't implement __index__.

@AlexWaygood
Copy link
Member

I also agree that this is a bad idea, for the same reasons that others have already listed.

@AlexWaygood AlexWaygood closed this as not planned Won't fix, can't repro, duplicate, stale Mar 18, 2023
@chrisnovakovic
Copy link
Author

Sounds like there are multiple good reasons not to do this - thanks for the input, everyone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-feature A feature request or enhancement
Projects
None yet
Development

No branches or pull requests

7 participants