Skip to content
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

_decimal and _pydecimal compatibility differences #117056

Open
antonok-edm opened this issue Mar 20, 2024 · 3 comments
Open

_decimal and _pydecimal compatibility differences #117056

antonok-edm opened this issue Mar 20, 2024 · 3 comments
Labels
type-bug An unexpected behavior, bug, or error

Comments

@antonok-edm
Copy link

antonok-edm commented Mar 20, 2024

Bug report

Bug description:

There are two backends for the decimal module, one from libmpdec (_decimal) and the other being a native re-implementation in Python (_pydecimal).

The signatures of methods like Decimal.__add__ take a named context argument when imported from _pydecimal, but cannot accept that argument when imported from _decimal.

mpdecimal is not bundled with Python installations on all platforms, and my understanding is that there are even more changes to how it's packaged incoming as per #115119. The difference between the two backends has caused cross-platform incompatibility for at least one project, and I'd argue it's very difficult for anyone who encounters this problem to understand the issue because the error messages mention nothing about an external dependency being required.

Would it be possible to remove the context argument from the _pydecimal backend, or to add it to the _decimal backend even if it's unused, to resolve the incompatibility?

CPython versions tested on:

3.11

Operating systems tested on:

Linux

@serhiy-storchaka
Copy link
Member

It is other way: the Python implementation accepts and passes the context argument to some special methods, while the C implementation has standard signatures.

The implementation is not designed for overloading these special methods in subclasses. Perhaps it would be better to implement the logic of addition in other method (e.g. _add()) or global function which accepts the context argument, and call it in __add__(), __sub__(), and other methods. It still not guarantee that Decimal subclasses with overloaded special methods work the same in both implementations, but it is closer to the C implementation.

cc @mdickinson

@antonok-edm
Copy link
Author

It is other way: the Python implementation accepts and passes the context argument to some special methods, while the C implementation has standard signatures.

Ah, right, I managed to get it backwards 😅
Updated the original post.

@rhettinger
Copy link
Contributor

rhettinger commented Mar 23, 2024

FWIW the extra context parameter was considered an implementation detail from the outset. It was not a documented part of the public API. In practice this was worked out well and proven harmless. The users likely never see it because arithmetic dunder methods aren't typically called directly (the + operator is more convenient and it supports the fallback to __radd__).

I recommend leaving this as-is. There isn't anything worth churning all this long stable code. The referenced issue in the rp2 project is rather exotic and not typically for a user of the decimal module.

If anything, I would suggest adding a note to the docs acknowledging the implementation detail.

Perhaps it would be better to implement the logic of addition in other method (e.g. _add()) or global function which accepts the context argument, and call it in add(), sub(), and other methods

This would be a lot of code churn for almost no benefit. It would make the module even harder to read and maintain. Also, in the pure python verion, and extra layer of function call forwarding would just may the module slower.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

3 participants