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

gh-85988: Change documentation for sys.float_info.rounds #99675

Merged
merged 5 commits into from
Nov 27, 2022

Conversation

tbwolfe
Copy link
Contributor

@tbwolfe tbwolfe commented Nov 22, 2022

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.

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.
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.
…cpython into float-info-rounds-documentation

Merge upstream branch into local topic branch
@mdickinson
Copy link
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.

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
Copy link
Contributor Author

tbwolfe commented Nov 22, 2022

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.

Copy link
Member

@mdickinson mdickinson left a comment

Choose a reason for hiding this comment

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

LGTM too. Thanks!

@mdickinson mdickinson merged commit 6562939 into python:main Nov 27, 2022
hugovk pushed a commit to hugovk/cpython that referenced this pull request Feb 28, 2023
…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 pushed a commit to hugovk/cpython that referenced this pull request Feb 28, 2023
…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.
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 news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants