gh-122450: Expand documentation for Rational and Fraction
#136800
Conversation
skirpichev
changed the title
skirpichev
added
needs backport to 3.13
There was a problem hiding this comment.
Look also at the paragraph before the constructor description: "A Fraction instance can be constructed from a pair of integers, from another rational number, or from a string." It is outdated. The numbers in the pair can be not only integers, and the single numerical argument can be not only rational. Making it more precise will only lead to repetition, so I would make it more generic instead.
| The first version requires that *numerator* and *denominator* are instances | ||
| of :class:`numbers.Rational` and returns a new :class:`Fraction` instance | ||
| with value equal to ``numerator/denominator`` where the denominator is positive. | ||
| If *denominator* is ``0``, it raises a :exc:`ZeroDivisionError`. | ||
| with value equal to ``numerator/denominator``. If *denominator* is zero, it | ||
| raises a :exc:`ZeroDivisionError`. |
There was a problem hiding this comment.
I still think this is worse. We now loose information that the denominator is positive, and implicitly require the user to know that {Fraction,Rational}.denominator should be positive. We are able to guarantee here that the denominator is a non-zero positive integer, there's no reason why we can't say that.
How about:
... and returns a new :class:`Fraction` instance with a value equal
to ``numerator/denominator``. The numerator and denominator of this
instance will be in the lowest terms, and the denominator will be positive.
There was a problem hiding this comment.
We now loose information that the denominator is positive, and implicitly require the user to know that {Fraction,Rational}.denominator should be positive.
No. I think that class constructor should describe it's construction, not attributes, right?
- This information will be present in descriptions of numerator/denominator attributes of the Fraction
- Or in Rational docs.
I would prefer (2) for less redundancy. But as you insist on restoring (1) - I'll do that.
|
A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated. Once you have made the requested changes, please leave a comment on this pull request containing the phrase |
AA-Turner
changed the title
Rational and Fraction
Co-authored-by: Adam Turner <9087854+AA-Turner@users.noreply.github.com>
|
I have made the requested changes; please review again |
|
Thanks for making the requested changes! @AA-Turner: please review the changes made to this pull request. |
Co-authored-by: Hugo Sansaqua privet.kitty99@gmail.com
denominatorofFractionis positive, which should be documented #122450Links: