Skip to content

gh-122450: Fix docs to state denominator positivity of Fraction #122464

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.

Sign up for GitHub

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
wants to merge 12 commits into from
Closed

gh-122450: Fix docs to state denominator positivity of Fraction #122464

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
04056ff
Fix manual to state denominator positivity of Fraction
privet-kitty Jul 30, 2024
e5a4608
Update Doc/library/fractions.rst
privet-kitty Jul 31, 2024
05c764d
Add docstrings to numerator and denominator of Fraction
privet-kitty Jul 31, 2024
ad66afb
Correct a word
privet-kitty Jul 31, 2024
416b9cd
Update Lib/fractions.py
privet-kitty Jul 31, 2024
7cb1905
Update Lib/fractions.py
privet-kitty Jul 31, 2024
ddec59c
Update Lib/fractions.py
privet-kitty Aug 1, 2024
ed48fb7
Update Doc/library/fractions.rst
privet-kitty Aug 1, 2024
c575b47
Add docstrings to numerator and denominator of Rational
privet-kitty Aug 1, 2024
a531cb6
Update Lib/fractions.py
privet-kitty Aug 2, 2024
1f2519a
Update Lib/fractions.py
privet-kitty Aug 2, 2024
2e07bfc
Merge branch 'main' into feature/denominator-positivity
AA-Turner Jul 19, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 2 additions & 3 deletions Doc/library/fractions.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,12 +121,11 @@ another rational number, or from a string.

.. attribute:: numerator

Numerator of the Fraction in lowest term.
Numerator of the Fraction in lowest terms.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Numerator of the Fraction in lowest terms.
Numerator of the Fraction, expressed in its lowest terms.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In fact, we say that "The Fraction class inherits from the abstract base class numbers.Rational, and implements all of the methods and operations from that class."

So, we could move these numerator/denominator descriptions to the Rational class.

CC @AA-Turner

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That would be worse; it requires people to look in two places to find out what happens with Fraction. Happy to improve in both places, but we shouldn't remove from here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pr continued in #136800. I hope, I'll address review quickly;-)

but we shouldn't remove from here.

I'm not sure if it worth verbosity. I don't see e.g. where described real/imag properties for builtin types, except for the numbers module.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skirpichev should this PR (#122464) be closed?

A

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so. I tried to attribute this work in a follow-up pr.


.. attribute:: denominator

Denominator of the Fraction in lowest term.

Positive denominator of the Fraction in lowest terms.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Positive denominator of the Fraction in lowest terms.
Positive denominator of the Fraction, expressed in its lowest terms.


.. method:: as_integer_ratio()

Expand Down
2 changes: 2 additions & 0 deletions Lib/numbers.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -297,11 +297,13 @@ class Rational(Real):
@property
@abstractmethod
def numerator(self):
"""The numerator of a rational number in lowest terms."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""The numerator of a rational number in lowest terms."""
"""The numerator of a rational number, expressed in its lowest terms."""

raise NotImplementedError

@property
@abstractmethod
def denominator(self):
"""The (positive) denominator of a rational number in lowest terms."""
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
"""The (positive) denominator of a rational number in lowest terms."""
"""The (positive) denominator of a rational number, expressed in its lowest terms."""

raise NotImplementedError

# Concrete implementation of Real's conversion to float.
Expand Down
Loading