Skip to content

Improve pow(fraction.Fraction(), b, modulo) error message #119594

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
wimglenn opened this issue May 27, 2024 · 3 comments
Closed

Improve pow(fraction.Fraction(), b, modulo) error message #119594

wimglenn opened this issue May 27, 2024 · 3 comments
Labels
type-feature A feature request or enhancement

Comments

@wimglenn
Copy link
Contributor

wimglenn commented May 27, 2024

Bug report

Bug description:

Fraction.__pow__ does not accept the third argument which pow tries to pass to it:

>>> pow(Fraction(1), 1, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Fraction.__pow__() takes 2 positional arguments but 3 were given

Other number types don't do that, they support it when they can and print an error message if they don't want to, e.g.

>>> pow(Decimal("1"), 1, 1)
Decimal('0')
>>> pow(Decimal('1'), 1.1, 1)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: pow() 3rd argument not allowed unless all arguments are integers

I'm not sure what fraction should do, maybe it should just continue not to support it? Or just accept the argument, and then print a better error message if modulo is not None?

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Linked PRs

@wimglenn wimglenn added the type-bug An unexpected behavior, bug, or error label May 27, 2024
@terryjreedy terryjreedy added type-feature A feature request or enhancement and removed type-bug An unexpected behavior, bug, or error labels May 27, 2024
@terryjreedy
Copy link
Member

Fractions.__pow__ should imitate float and Decimal __pow__.

pow(3.0, 7, 3)
Traceback (most recent call last):
  File "<pyshell#5>", line 1, in <module>
    pow(3.0, 7, 3)
TypeError: pow() 3rd argument not allowed unless all arguments are integers

Since the exception type is correct, this should be considered an enhancement rather than bug.

PR was a bit premature without agreement of what fix should be.

@mdickinson
Copy link
Member

Not supporting a modulus was an intentional choice here.

It's hard to imagine real use-cases would be for supporting the modulus. If m is a positive integer, the collection of integers modulo m has nice arithmetic properties (it's a commutative unital ring), and exponentiation modulo m is a reasonably common operation (e.g., in cryptography). That's no longer true for the collection of fractions modulo m - that set doesn't have an associative multiplication operation, so a power operation is at best ill-defined.

If we want to change the error message (as @terryjreedy pointed out, the exception type is already the right one), the first thing to try would be to return NotImplemented if a third argument is given. That should work, but the special-casing for __pow__ is messy enough that it might not.

@mdickinson
Copy link
Member

mdickinson commented May 27, 2024

That should work, but [...]

It does. Results of a quick test:

>>> pow(Fraction(2), 3)
Fraction(8, 1)
>>> pow(Fraction(2), 3, 5)
Traceback (most recent call last):
  File "<python-input-4>", line 1, in <module>
    pow(Fraction(2), 3, 5)
    ~~~^^^^^^^^^^^^^^^^^^^
TypeError: unsupported operand type(s) for ** or pow(): 'Fraction', 'int', 'int'

@terryjreedy terryjreedy changed the title fractions.Fraction.__pow__ can receive 3rd argument from builtin function pow Improve pow(fraction.Fraction(), b, modulo) error message May 29, 2024
terryjreedy pushed a commit that referenced this issue May 29, 2024
…119593)

If one calls pow(fractions.Fraction, x, module) with modulo not None, the error message now says that the types are incompatible rather than saying pow only takes 2 arguments.  Implemented by having fractions.Fraction __pow__ accept optional modulo argument and return NotImplemented if not None.  pow() then raises with appropriate message.
---------

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
noahbkim pushed a commit to hudson-trading/cpython that referenced this issue Jul 11, 2024
…ssage (python#119593)

If one calls pow(fractions.Fraction, x, module) with modulo not None, the error message now says that the types are incompatible rather than saying pow only takes 2 arguments.  Implemented by having fractions.Fraction __pow__ accept optional modulo argument and return NotImplemented if not None.  pow() then raises with appropriate message.
---------

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
estyxx pushed a commit to estyxx/cpython that referenced this issue Jul 17, 2024
…ssage (python#119593)

If one calls pow(fractions.Fraction, x, module) with modulo not None, the error message now says that the types are incompatible rather than saying pow only takes 2 arguments.  Implemented by having fractions.Fraction __pow__ accept optional modulo argument and return NotImplemented if not None.  pow() then raises with appropriate message.
---------

Co-authored-by: Mark Dickinson <dickinsm@gmail.com>
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

3 participants