Skip to content
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

Replace numbers.Real with Real in numbers.rst #31995

Merged
merged 1 commit into from May 10, 2022

Conversation

maggyero
Copy link
Contributor

@maggyero maggyero commented Mar 19, 2022

No description provided.

@bedevere-bot bedevere-bot added the docs Documentation in the Doc dir label Mar 19, 2022
@sweeneyde

This comment has been hidden.

@maggyero
Copy link
Contributor Author

maggyero commented Mar 19, 2022

I believe that since we're in the numbers module docs, we can assume that Real means numbers.Real, and Complex means numbers.Complex. In other words, you can assume that from numbers import Real, Complex has already happened.

Yes we can assume it, that is precisely why I opened this pull request, since this is the only part of the code snippet which does not make that assumption.

@sweeneyde
Copy link
Member

sweeneyde commented Mar 19, 2022

Ah, sorry I misunderstood.

@sweeneyde sweeneyde changed the title Update numbers.rst Replace numbers.Real with Real in numbers.rst Mar 19, 2022
@JelleZijlstra
Copy link
Member

JelleZijlstra commented Mar 28, 2022

Not sure about this, since it's supposed to be an example of how to use numbers, and the real code it's based on does use numbers.Real, not Real.

@maggyero
Copy link
Contributor Author

maggyero commented Mar 29, 2022

Not sure about this, since it's supposed to be an example of how to use numbers, and the real code it's based on does use numbers.Real, not Real.

The real code does not use if isinstance(a, Rational): but if isinstance(a, numbers.Rational): so the current numbers documentation is not even consistent with the real code:

https://github.com/python/cpython/blob/v3.10.4/Lib/fractions.py#L368-L377

Copy link
Contributor

@MaxwellDupre MaxwellDupre left a comment

Looks better.

@rhettinger
Copy link
Contributor

rhettinger commented May 10, 2022

Closing and reopening to trigger the bots.

@rhettinger rhettinger closed this May 10, 2022
@rhettinger rhettinger reopened this May 10, 2022
@rhettinger rhettinger merged commit dde8a16 into python:main May 10, 2022
22 checks passed
@maggyero
Copy link
Contributor Author

maggyero commented May 10, 2022

Thanks for the review!

@miss-islington
Copy link
Contributor

miss-islington commented May 11, 2022

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

@bedevere-bot
Copy link

bedevere-bot commented May 11, 2022

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

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 11, 2022
(cherry picked from commit dde8a16)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
miss-islington added a commit that referenced this pull request May 11, 2022
(cherry picked from commit dde8a16)

Co-authored-by: Géry Ogam <gery.ogam@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Documentation in the Doc dir skip issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants