gh-121797: Add class method Fraction.from_number()#121800
gh-121797: Add class method Fraction.from_number()#121800serhiy-storchaka merged 5 commits intopython:mainfrom
Conversation
It is an alternative constructor which only accepts a single numeric argument. Unlike to Fraction.from_float() and Fraction.from_decimal() it accepts any real numbers supported by the standard constructor (int, float, Decimal, Rational numbers). Unlike to the standard constructor, it does not accept strings.
mdickinson
left a comment
There was a problem hiding this comment.
+1 for the general idea. I haven't reviewed line-by-line.
skirpichev
left a comment
There was a problem hiding this comment.
LGTM with one nitpick (feel free to ignore)
| return self | ||
|
|
||
| @classmethod | ||
| def from_number(cls, number): |
There was a problem hiding this comment.
Hmm, you can reuse this in the RationalFraction constructor, isn't? First three if/elif's share exactly same logic.
There was a problem hiding this comment.
Do you mean the main Fraction constructor?
There is a different exception if the argument type is not supported. There is also a matter of performance.
There was a problem hiding this comment.
I meant some helper:
@classmethod
def _from_number(cls, number):
if type(number) is int:
return cls._from_coprime_ints(number, 1)
elif isinstance(number, numbers.Rational):
return cls._from_coprime_ints(number.numerator, number.denominator)
elif (isinstance(number, float) or
(not isinstance(number, type) and
hasattr(number, 'as_integer_ratio'))):
return cls._from_coprime_ints(*number.as_integer_ratio())Then you can reuse one in the constructor and in the from_number(), e.g.:
if denominator is None:
self = cls._from_number(numerator)
if self is not None:
return self
elif isinstance(numerator, str):
...
There is also a matter of performance.
I don't expect too much from extra class method call.
There was a problem hiding this comment.
Extra function call and extra check.
This will not save much code. I'll leave this on future if we touch this code again.
There was a problem hiding this comment.
Well, it's up to you. PR has Mark approval, so it's ready to go.
It is an alternative constructor which only accepts a single numeric argument. Unlike to Fraction.from_float() and Fraction.from_decimal() it accepts any real numbers supported by the standard constructor (int, float, Decimal, Rational numbers).
Unlike to the standard constructor, it does not accept strings.
📚 Documentation preview 📚: https://cpython-previews--121800.org.readthedocs.build/