gh-92417: `fractions`, `decimal`: Improve docs for alternative constructor methods by AlexWaygood · Pull Request #92421 · python/cpython

AlexWaygood · 2022-05-07T11:59:04Z

serhiy-storchaka

I would remove the note directive at all. It attracts too much attention. Instead I would add something like "This is a limited version of the :class:`Fraction` constructor which only accepts :class:`~numbers.Integral` or :class:`float`." in plain paragraph.

Replace the method directive with classmethod for class methods.

And the same should be done for Decimal. It would be better to do all this in a single PR.

Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

serhiy-storchaka · 2022-05-09T05:17:54Z

It would be nice if some math people (like @mdickinson, or @rhettinger, or @tim-one, or @skrah) take a look at it.

mdickinson

One nitpick; otherwise, the changes look good to me.

bedevere-bot · 2022-05-09T20:22:41Z

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 I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

And if you don't make the requested changes, you will be put in the comfy chair!

AlexWaygood · 2022-05-10T11:52:26Z

Thanks all for the reviews! I feel like I've received slightly contradictory feedback on this, and it was certainly never my intent to make an aggressive edit, so I've reduced the PR down to a minimal diff which should hopefully be less controversial.

I have made the requested changes; please review again.

bedevere-bot · 2022-05-10T11:52:30Z

Thanks for making the requested changes!

@serhiy-storchaka, @mdickinson, @rhettinger: please review the changes made to this pull request.

mdickinson · 2022-05-10T18:36:11Z

I feel like I've received slightly contradictory feedback on this

Agreed. Raymond's assigned this to himself, so I'll duck out at this point and leave this to you and him.

AlexWaygood · 2022-05-18T18:02:48Z

@rhettinger, is there anything more you'd like me to do here? :)

(I don't have merge privileges; I'm only a triager.)

miss-islington · 2022-05-19T15:28:54Z

Thanks @AlexWaygood for the PR, and @ambv for merging it 🌮🎉.. I'm working now to backport this PR to: 3.10, 3.11.
🐍🍒⛏🤖

bedevere-bot · 2022-05-19T15:29:06Z

GH-92970 is a backport of this pull request to the 3.11 branch.

…constructor methods (pythonGH-92421) Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com> (cherry picked from commit 090df84) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

bedevere-bot · 2022-05-19T15:29:16Z

GH-92971 is a backport of this pull request to the 3.10 branch.

…uctor methods (GH-92421) (GH-92970) Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com> (cherry picked from commit 090df84) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

…uctor methods (GH-92421) (GH-92971) Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com> (cherry picked from commit 090df84) Co-authored-by: Alex Waygood <Alex.Waygood@Gmail.com>

pythongh-92417:fractionsdocs: remove references to Python <3.2
453bc8c
python#92417

bedevere-bot added docs Documentation in the Doc dir awaiting review labels May 7, 2022

AlexWaygood added the skip news label May 7, 2022

AlexWaygood mentioned this pull request May 7, 2022
Many references to unsupported Python versions in the stdlib docs #92417
Closed

AlexWaygood added needs backport to 3.9 needs backport to 3.10 only security fixes labels May 7, 2022

serhiy-storchaka reviewed May 8, 2022
View reviewed changes

AlexWaygood marked this pull request as draft May 8, 2022 05:05

AlexWaygood added the needs backport to 3.11 only security fixes label May 8, 2022

Address review
eb5a0ac

AlexWaygood changed the title ~~gh-92417: fractions docs: remove references to Python <3.2~~gh-92417: fractions, decimal: Improve docs for alternative constructor methodsMay 8, 2022

reST
ac23bec

serhiy-storchaka reviewed May 8, 2022
View reviewed changes

Comment threadDoc/library/fractions.rst Outdated
Comment threadDoc/library/decimal.rst Outdated

serhiy-storchaka requested a review from mdickinson May 8, 2022 17:24

Address review 2
20399c3

AlexWaygood marked this pull request as ready for review May 8, 2022 17:44

AlexWaygood requested a review from rhettinger as a code owner May 8, 2022 17:44

AlexWaygood requested a review from serhiy-storchaka May 8, 2022 17:50

ezio-melotti reviewed May 8, 2022
View reviewed changes

Comment threadDoc/library/decimal.rst Outdated

More doctest
f519d0a
Co-authored-by: Ezio Melotti <ezio.melotti@gmail.com>

mdickinson reviewed May 9, 2022
View reviewed changes

Comment threadDoc/library/decimal.rst Outdated

mdickinson reviewed May 9, 2022
View reviewed changes

Comment threadDoc/library/decimal.rst

AlexWaygood added 2 commits May 9, 2022 08:41

Use longform representation, not hex representation
e023dc6

Add notes that these are legacy methods
8f537ea

mdickinson reviewed May 9, 2022
View reviewed changes

Comment threadDoc/library/decimal.rst Outdated

I can write in full sentences
4435772

rhettinger self-assigned this May 9, 2022

rhettinger requested changes May 9, 2022
View reviewed changes

Comment threadDoc/library/decimal.rst Outdated
Comment threadDoc/library/decimal.rst
Comment threadDoc/library/decimal.rst
Comment threadDoc/library/fractions.rst

bedevere-bot removed the awaiting merge label May 9, 2022

bedevere-bot added the awaiting changes label May 9, 2022

Start again
bf19119

bedevere-bot added awaiting change review and removed awaiting changes labels May 10, 2022

bedevere-bot requested review from mdickinson, rhettinger and serhiy-storchaka May 10, 2022 11:52

mdickinson removed their request for review May 10, 2022 18:36

rhettinger approved these changes May 13, 2022
View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels May 13, 2022

ambv merged commit 090df84 into python:mainMay 19, 2022

bedevere-bot removed the awaiting merge label May 19, 2022

bedevere-bot removed the needs backport to 3.11 only security fixes label May 19, 2022

bedevere-bot removed the needs backport to 3.10 only security fixes label May 19, 2022

AlexWaygood deleted the patch-5 branch May 19, 2022 15:29

Uh oh!

Uh oh!

Conversation

AlexWaygood commented May 7, 2022

Uh oh!

serhiy-storchaka left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

serhiy-storchaka commented May 9, 2022

Uh oh!

Uh oh!

mdickinson left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented May 9, 2022

Uh oh!

AlexWaygood commented May 10, 2022

Uh oh!

bedevere-bot commented May 10, 2022

Uh oh!

mdickinson commented May 10, 2022

Uh oh!

AlexWaygood commented May 18, 2022

Uh oh!

miss-islington commented May 19, 2022

Uh oh!

bedevere-bot commented May 19, 2022

Uh oh!

bedevere-bot commented May 19, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants