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-85988: Change documentation for sys.float_info.rounds by tbwolfe · Pull Request #99675 · python/cpython

Conversation

@tbwolfe

@tbwolfe tbwolfe commented Nov 22, 2022 •
edited by bedevere-bot
Loading

Uh oh!

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

Copy link
Copy Markdown
Contributor

Change the documentation for sys.float_info.rounds to remove references to C99 section 5.2.4.2.2 and instead place the available values inline.

  • Issue: Document the meaning of values for sys.float_info.rounds #85988

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@tbwolfe
Change documentation for sys.float_info.rounds
a724de0 
Change the documentation for sys.float_info.rounds to remove
references to C99 section 5.2.4.2.2 and instead place the
available values inline.
@bedevere-bot bedevere-bot mentioned this pull request Nov 22, 2022
Document the meaning of values for sys.float_info.rounds #85988
Closed
@bedevere-bot bedevere-bot added awaiting review docs Documentation in the Doc dir skip news labels Nov 22, 2022
tbwolfe added 3 commits November 22, 2022 08:13
@tbwolfe
Merge branch 'main' into float-info-rounds-documentation
49c9296
@tbwolfe
Correction to previous documentation change
7031f9f 
Newlines were not preserved in generated HTML on previous
commit. I have changes the list to a comma-separated list
of values and their meanings.
@tbwolfe
Merge branch 'float-info-rounds-documentation' of github.com:tbwolfe/…
51a2d35 
…cpython into float-info-rounds-documentation

Merge upstream branch into local topic branch
@mdickinson

mdickinson commented Nov 22, 2022

Copy link
Copy Markdown
Member

Thanks for the PR. A couple of comments:

We should keep the "This reflects the value of the system FLT_ROUNDS macro at interpreter startup time" text: there's important information there. In C, FLT_ROUNDS is the one float.h value that's not necessarily constant, potentially reflecting the current rounding mode. But Python's value is constant, and won't change if the rounding mode is changed dynamically. (With hindsight, I suspect that sys.float_info.rounds isn't all that useful, and would have been better left out of the struct - it's the only piece of information that relates to the semantics of operations rather than simply to the storage format. But it's too late to change that now.)

The "for floating-point addition" language seems curiously specific (yes, I know it comes directly from the standard). I'd expect the rounding mode to apply to all arithmetic operations (possibly excluding sqrt); not just addition. The C++ standard appears to use the language "floating-point arithmetic operations" instead.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@tbwolfe
Clarify source for value of FLT_ROUNDS
4243533 
Clarify the source of the FLT_ROUNDS value and
change 'floating-point addition' to 'floating-point
arithmetic' to indicate that the rounding mode
applies to all arithmetic operations.
@tbwolfe

tbwolfe commented Nov 22, 2022

Copy link
Copy Markdown
Contributor Author

Thank you for the feedback, @mdickinson ! I have made a few changes to address your points.

Just let me know if you see anything that I might have overlooked, and I'll be glad to sort it out.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
rhettinger
rhettinger approved these changes Nov 23, 2022
View reviewed changes
@bedevere-bot bedevere-bot added awaiting merge and removed awaiting review labels Nov 23, 2022
mdickinson
mdickinson approved these changes Nov 27, 2022
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 too. Thanks!

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@mdickinson
mdickinson merged commit 6562939 into python:main Nov 27, 2022
@bedevere-bot bedevere-bot removed the awaiting merge label Nov 27, 2022
hugovk pushed a commit to hugovk/cpython that referenced this pull request Feb 28, 2023
@tbwolfe @hugovk
pythongh-85988: Change documentation for sys.float_info.rounds (pytho…
dee1366 
…nGH-99675)

* Change documentation for sys.float_info.rounds

Change the documentation for sys.float_info.rounds to remove
references to C99 section 5.2.4.2.2 and instead place the
available values inline.

* Correction to previous documentation change

Newlines were not preserved in generated HTML on previous
commit. I have changes the list to a comma-separated list
of values and their meanings.

* Clarify source for value of FLT_ROUNDS

Clarify the source of the FLT_ROUNDS value and
change 'floating-point addition' to 'floating-point
arithmetic' to indicate that the rounding mode
applies to all arithmetic operations.
@hugovk hugovk mentioned this pull request Feb 28, 2023
[3.11] gh-90744: Fix erroneous doc links in the sys module (GH-101319) #102321
Merged
hugovk pushed a commit to hugovk/cpython that referenced this pull request Feb 28, 2023
@tbwolfe @hugovk
pythongh-85988: Change documentation for sys.float_info.rounds (pytho…
e6bf616 
…nGH-99675)

* Change documentation for sys.float_info.rounds

Change the documentation for sys.float_info.rounds to remove
references to C99 section 5.2.4.2.2 and instead place the
available values inline.

* Correction to previous documentation change

Newlines were not preserved in generated HTML on previous
commit. I have changes the list to a comma-separated list
of values and their meanings.

* Clarify source for value of FLT_ROUNDS

Clarify the source of the FLT_ROUNDS value and
change 'floating-point addition' to 'floating-point
arithmetic' to indicate that the rounding mode
applies to all arithmetic operations.
This was referenced Feb 28, 2023
[3.10] gh-90744: Fix erroneous doc links in the sys module (GH-101319) #102322
Merged
GH-90744: Fix erroneous doc links in the sys module #101319
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@rhettinger rhettinger rhettinger approved these changes
+1 more reviewer
@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.

4 participants

@tbwolfe @mdickinson @rhettinger @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.