Skip to content

gh-101575: document Decimal.__round__() #101737

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 8 commits into from
Jun 12, 2024
Merged

gh-101575: document Decimal.__round__() #101737

merged 8 commits into from
Jun 12, 2024

Conversation

OTheDev
Copy link
Contributor

@OTheDev OTheDev commented Feb 9, 2023 •
edited by bedevere-bot
Loading

@OTheDev OTheDev requested a review from rhettinger as a code owner February 9, 2023 11:06
@bedevere-bot bedevere-bot mentioned this pull request Feb 9, 2023
Closed
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir skip news labels Feb 9, 2023
@blaisep
Copy link
Contributor

blaisep commented May 20, 2024

@encukou , is this something we could merge?
I noticed that the topic is still absent after https://docs.python.org/3/library/decimal.html#decimal.Decimal.to_integral_value

@encukou
Copy link
Member

encukou commented May 20, 2024

Thanks for the mention.
The issue here is that we don't want people to call d.__round__() directly; rather they should do round(d), as in the examples. The double-underscore method is for defining the behaviour, not using it; the docs are for users.

I'm not sure how this should be best documented; perhaps describe:: as used for set operations.

@encukou
Copy link
Member

encukou commented May 21, 2024

After talking to a few docs people, I'm pretty sure .. describe:: is the way to document this.

blaisep
blaisep suggested changes May 21, 2024
View reviewed changes
picnixz
picnixz reviewed May 24, 2024
View reviewed changes
Copy link
Member

@picnixz picnixz left a comment

Choose a reason for hiding this comment

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

Some wording suggestions

@encukou
Copy link
Member

encukou commented Jun 4, 2024

Looking at the rendered output, I saw the describe looks exactly like a method, which might be confusing.
I added an introduction line to separate them. Since i had the change locally to test it, I took the liberty to push the suggestion to this PR; I hope you don't mind.

Does this look good?

image

@picnixz
Copy link
Member

picnixz commented Jun 4, 2024

Generally, you'd use .. describe with a "legal" expression, e.g., .. describe:: x in S for the set membership where x and S are explained in a subsequent paragraph. In our case, I would suggest using something like:

.. describe:: round(x)
.. describe:: round(x, n)

Using a signature-based description would indeed be more confusing IMO.

@encukou
Copy link
Member

encukou commented Jun 10, 2024

Well, it's not less clear that way, so let's go with it?
image

@OTheDev
Copy link
Contributor Author

OTheDev commented Jun 11, 2024

@encukou @picnixz Thanks! Your suggestions/changes LGTM!

@encukou
Copy link
Member

encukou commented Jun 12, 2024

Thank you for the fix, @OTheDev!

@encukou encukou merged commit 7dd8c37 into python:main Jun 12, 2024
25 checks passed
@encukou encukou added needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Jun 12, 2024
@miss-islington-app
Copy link

Thanks @OTheDev for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13.
🐍🍒⛏🤖

@miss-islington-app
Copy link

Thanks @OTheDev for the PR, and @encukou for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 12, 2024
@OTheDev @miss-islington
pythongh-101575: document Decimal.__round__() (pythonGH-101737)
f1e70e5 
(cherry picked from commit 7dd8c37)

Co-authored-by: Owain Davies <[email protected]>
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 12, 2024
@OTheDev @miss-islington
pythongh-101575: document Decimal.__round__() (pythonGH-101737)
2d69001 
(cherry picked from commit 7dd8c37)

Co-authored-by: Owain Davies <[email protected]>
@bedevere-app
Copy link

bedevere-app bot commented Jun 12, 2024

GH-120394 is a backport of this pull request to the 3.13 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.13 bugs and security fixes label Jun 12, 2024
@bedevere-app
Copy link

bedevere-app bot commented Jun 12, 2024

GH-120395 is a backport of this pull request to the 3.12 branch.

@bedevere-app bedevere-app bot removed the needs backport to 3.12 only security fixes label Jun 12, 2024
encukou pushed a commit that referenced this pull request Jun 13, 2024
@miss-islington @OTheDev
[3.13] gh-101575: document Decimal.__round__() (GH-101737) (GH-120394)
b1ebbb5 
gh-101575: document Decimal.__round__() (GH-101737)
(cherry picked from commit 7dd8c37)

Co-authored-by: Owain Davies <[email protected]>
encukou pushed a commit that referenced this pull request Jun 13, 2024
@miss-islington @OTheDev
[3.12] gh-101575: document Decimal.__round__() (GH-101737) (GH-120395)
8952323 
gh-101575: document Decimal.__round__() (GH-101737)
(cherry picked from commit 7dd8c37)

Co-authored-by: Owain Davies <[email protected]>
mrahtz pushed a commit to mrahtz/cpython that referenced this pull request Jun 30, 2024
@OTheDev @mrahtz
pythongh-101575: document Decimal.__round__() (pythonGH-101737)
6335b27
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
@OTheDev @noahbkim
pythongh-101575: document Decimal.__round__() (pythonGH-101737)
c7edbf9
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
@OTheDev @estyxx
pythongh-101575: document Decimal.__round__() (pythonGH-101737)
e579a8c
@OTheDev OTheDev deleted the fix-issue-101575 branch January 28, 2025 03:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@encukou encukou encukou left review comments

@picnixz picnixz picnixz left review comments

@blaisep blaisep blaisep requested changes

@rhettinger rhettinger Awaiting requested review from rhettinger

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.
5 participants
@OTheDev @blaisep @encukou @picnixz @bedevere-bot