-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
bpo-37863: Optimize Fraction.__hash__() #15298
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
Changes from all commits
69aab95
7d9079c
54b8eb9
70903b4
e4de515
7cb3c5d
6c32213
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -556,23 +556,19 @@ def __round__(self, ndigits=None): | |
def __hash__(self): | ||
"""hash(self)""" | ||
|
||
# XXX since this method is expensive, consider caching the result | ||
|
||
# In order to make sure that the hash of a Fraction agrees | ||
# with the hash of a numerically equal integer, float or | ||
# Decimal instance, we follow the rules for numeric hashes | ||
# outlined in the documentation. (See library docs, 'Built-in | ||
# Types'). | ||
|
||
# dinv is the inverse of self._denominator modulo the prime | ||
# _PyHASH_MODULUS, or 0 if self._denominator is divisible by | ||
# _PyHASH_MODULUS. | ||
dinv = pow(self._denominator, _PyHASH_MODULUS - 2, _PyHASH_MODULUS) | ||
if not dinv: | ||
# To make sure that the hash of a Fraction agrees with the hash | ||
# of a numerically equal integer, float or Decimal instance, we | ||
# follow the rules for numeric hashes outlined in the | ||
# documentation. (See library docs, 'Built-in Types'). | ||
|
||
try: | ||
dinv = pow(self._denominator, -1, _PyHASH_MODULUS) | ||
except ValueError: | ||
# ValueError means there is no modular inverse | ||
hash_ = _PyHASH_INF | ||
else: | ||
hash_ = abs(self._numerator) * dinv % _PyHASH_MODULUS | ||
result = hash_ if self >= 0 else -hash_ | ||
hash_ = hash(abs(self._numerator)) * dinv % _PyHASH_MODULUS | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Another possibility is to write this as
to avoid the (in general) 122-by-61 bit |
||
result = hash_ if self._numerator >= 0 else -hash_ | ||
return -2 if result == -1 else result | ||
|
||
def __eq__(a, b): | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Optimizations for Fraction.__hash__ suggested by Tim Peters. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In what case
ValueError
is raised?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ValueError
is raised if and only if_PyHASH_MODULUS
dividesself._denominator
. There is no modular inverse if and only if the gcd of the base and the modulus is non-trivial (not 1), but the modulus is a prime in this case, so in this case that's the same as the base being a multiple of the modulus.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you. We need a test for this.