GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — gh-101825: mention, that as_integer_ratio() output is normalized by skirpichev · Pull Request #101843 · python/cpython

Conversation

@skirpichev

@skirpichev skirpichev commented Feb 12, 2023 •
edited by bedevere-bot
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member
  • Issue: as_integer_ratio docstring's #101825

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added the awaiting review label Feb 12, 2023
@bedevere-bot bedevere-bot mentioned this pull request Feb 12, 2023
as_integer_ratio docstring's #101825
Closed
@skirpichev

skirpichev commented Feb 12, 2023

Copy link
Copy Markdown
Member Author

For some context:

  • int.as_integer_ratio() was added in bpo-33073: Adding as_integer_ratio to ints. #8750
  • Fraction.as_integer_ratio() was added in bpo-37819: Add Fraction.as_integer_ratio() #15212
  • float.as_integer_ratio() was added in 2685563, return normilized pair since 292d351
  • Decimal.as_integer_ratio() was added in 53f2e0a
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@arhadthedev arhadthedev added docs Documentation in the Doc dir skip news labels Feb 12, 2023
@skirpichev
pythongh-101825: mention, that as_integer_ratio() output is normalized
d3de603
@skirpichev
skirpichev force-pushed the as_integer_ratio-docs branch from 9c54422 to d3de603 Compare February 12, 2023 07:46
OTheDev
OTheDev reviewed Feb 12, 2023
View reviewed changes
Comment thread Doc/library/stdtypes.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
OTheDev
OTheDev reviewed Feb 12, 2023
View reviewed changes
Comment thread Doc/library/stdtypes.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
skirpichev and others added 2 commits February 12, 2023 13:17
@skirpichev @OTheDev
Update Doc/library/stdtypes.rst
dc76383 
Co-authored-by: Owain Davies <[email protected]>
@skirpichev
Slightly adjust above suggestion and do same for the docstring
771db87
OTheDev
OTheDev approved these changes Feb 12, 2023
View reviewed changes

@OTheDev OTheDev left a comment •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

+1 from me!

One last nitpick (up to you though, of course) - I really like the language you used for float.as_integer_ratio() in floatobject.c.h/floatobject.c and would also like to see that reused in stdtypes.rst for better flow and consistency (: In particular, I think it reads better when split up into multiple, smaller sentences.

Overall, I think this is a positive change! Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting core review and removed awaiting review labels Feb 12, 2023
skirpichev
skirpichev commented Feb 12, 2023
View reviewed changes
Comment thread Objects/longobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
skirpichev added 2 commits February 13, 2023 07:58
@skirpichev
+1 (to cover bool, for instance)
19738a7
@skirpichev
Lets not test a patience of the reader with examples for this...
0aeb33a
@skirpichev

skirpichev commented Feb 13, 2023

Copy link
Copy Markdown
Member Author

@OTheDev, thanks for the review. Could you, please, take a look on last two commits?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@skirpichev
Amend 0aeb33a
f04dc7b
@skirpichev
skirpichev force-pushed the as_integer_ratio-docs branch from c33e88d to f04dc7b Compare February 13, 2023 06:16
@arhadthedev
arhadthedev requested review from ericsnowcurrently and mdickinson February 13, 2023 06:33
OTheDev
OTheDev reviewed Feb 13, 2023
View reviewed changes
Comment thread Doc/library/stdtypes.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
OTheDev
OTheDev reviewed Feb 13, 2023
View reviewed changes
Comment thread Objects/longobject.c Resolved

Uh oh!

There was an error while loading. Please reload this page.
OTheDev
OTheDev reviewed Feb 13, 2023
View reviewed changes

@OTheDev OTheDev left a comment

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

See specific comments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@ericsnowcurrently
ericsnowcurrently removed their request for review February 13, 2023 22:58
mdickinson
mdickinson reviewed Feb 26, 2023
View reviewed changes
Comment thread Doc/library/stdtypes.rst Resolved

Uh oh!

There was an error while loading. Please reload this page.
mdickinson
mdickinson reviewed Feb 26, 2023
View reviewed changes
Comment thread Lib/fractions.py Resolved

Uh oh!

There was an error while loading. Please reload this page.
skirpichev and others added 2 commits February 27, 2023 07:11
@skirpichev @mdickinson
Apply suggestions from code review
7892a40 
Co-authored-by: Mark Dickinson <[email protected]>
@skirpichev
Merge branch 'main' into as_integer_ratio-docs
c7dcff6
skirpichev added 3 commits February 27, 2023 07:25
@skirpichev
Revert int.as_integer_ration() docstring/rst docs to the original patch
fdb47d2
@skirpichev
Amend d3de603 (correct fractions.rst)
31a94d0
@skirpichev
Amend 7892a40 (same nitpick for float)
a2a84ae
@skirpichev
skirpichev requested a review from mdickinson February 27, 2023 05:26
@OTheDev

OTheDev commented Feb 27, 2023

Copy link
Copy Markdown
Contributor

This looks like a nice consolidation of the language used to describe as_integer_ratio(). The descriptions are now also more exact! 👍
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
mdickinson
mdickinson approved these changes Feb 27, 2023
View reviewed changes

@mdickinson mdickinson left a comment

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting core review labels Feb 27, 2023
@mdickinson
mdickinson merged commit 4624987 into python:main Feb 27, 2023
@bedevere-bot bedevere-bot removed the awaiting merge label Feb 27, 2023
@skirpichev
skirpichev deleted the as_integer_ratio-docs branch February 28, 2023 02:21
@skirpichev

skirpichev commented Feb 28, 2023

Copy link
Copy Markdown
Member Author

Thanks to all.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
carljm added a commit to carljm/cpython that referenced this pull request Feb 28, 2023
@carljm
Merge branch 'main' into inlinecomp2
73dc0ed 
* main: (67 commits)
  pythongh-99108: Add missing md5/sha1 defines to Modules/Setup (python#102308)
  pythongh-100227: Move _str_replace_inf to PyInterpreterState (pythongh-102333)
  pythongh-100227: Move the dtoa State to PyInterpreterState (pythongh-102331)
  pythonGH-102305: Expand some macros in generated_cases.c.h (python#102309)
  Migrate to new PSF mailgun account (python#102284)
  pythongh-102192: Replace PyErr_Fetch/Restore etc by more efficient alternatives (in Python/) (python#102193)
  pythonGH-90744: Fix erroneous doc links in the sys module (python#101319)
  pythongh-87092: Make jump target label equal to the offset of the target in the instructions sequence (python#102093)
  pythongh-101101: Unstable C API tier (PEP 689) (pythonGH-101102)
  IDLE: Simplify DynOptionsMenu __init__code (python#101371)
  pythongh-101561: Add typing.override decorator (python#101564)
  pythongh-101825: Clarify that as_integer_ratio() output is always normalized (python#101843)
  pythongh-101773: Optimize creation of Fractions in private methods (python#101780)
  pythongh-102251: Updates to test_imp Toward Fixing Some Refleaks (pythongh-102254)
  pythongh-102296 Document that inspect.Parameter kinds support ordering (pythonGH-102297)
  pythongh-102250: Fix double-decref in COMPARE_AND_BRANCH error case (pythonGH-102287)
  pythongh-101100: Fix sphinx warnings in `types` module (python#102274)
  pythongh-91038: Change default argument value to `False` instead of `0` (python#31621)
  pythongh-101765: unicodeobject: use Py_XDECREF correctly (python#102283)
  [doc] Improve grammar/fix missing word (pythonGH-102060)
  ...
scoder added a commit to scoder/quicktions that referenced this pull request Mar 19, 2023
@scoder
Improve docstring of Fraction.as_integer_ratio(), following python/cp…
fb7be90 
…ython#101843
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arhadthedev arhadthedev arhadthedev left review comments
+2 more reviewers
@OTheDev OTheDev OTheDev approved these changes
@mdickinson mdickinson mdickinson approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

5 participants

@skirpichev @OTheDev @mdickinson @arhadthedev @bedevere-bot
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.