Skip to content

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

Merged
encukou merged 8 commits into
python:mainfrom
OTheDev:fix-issue-101575
Jun 12, 2024
Merged

gh-101575: document Decimal.__round__()#101737
encukou merged 8 commits into
python:mainfrom
OTheDev:fix-issue-101575

Conversation

@OTheDev

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

Copy link
Copy Markdown
Contributor

@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

blaisep commented May 20, 2024

Copy link
Copy Markdown
Contributor

@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

encukou commented May 20, 2024

Copy link
Copy Markdown
Member

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

encukou commented May 21, 2024

Copy link
Copy Markdown
Member

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
Comment thread Doc/library/decimal.rst Outdated
picnixz
picnixz reviewed May 24, 2024
View reviewed changes

@picnixz picnixz left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Some wording suggestions

Comment thread Doc/library/decimal.rst Outdated
Comment thread Doc/library/decimal.rst Outdated
@encukou

encukou commented Jun 4, 2024

Copy link
Copy Markdown
Member

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

picnixz commented Jun 4, 2024

Copy link
Copy Markdown
Member

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

encukou commented Jun 10, 2024

Copy link
Copy Markdown
Member

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

@OTheDev

OTheDev commented Jun 11, 2024

Copy link
Copy Markdown
Contributor Author

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

@encukou

encukou commented Jun 12, 2024

Copy link
Copy Markdown
Member

Thank you for the fix, @OTheDev!

@encukou encukou merged commit 7dd8c37 into python:main Jun 12, 2024
@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

miss-islington-app Bot commented Jun 12, 2024

Copy link
Copy Markdown

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

@miss-islington-app

miss-islington-app Bot commented Jun 12, 2024

Copy link
Copy Markdown

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 <116417456+OTheDev@users.noreply.github.com>
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 <116417456+OTheDev@users.noreply.github.com>
@bedevere-app

bedevere-app Bot commented Jun 12, 2024

Copy link
Copy Markdown

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

bedevere-app Bot commented Jun 12, 2024

Copy link
Copy Markdown

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 <116417456+OTheDev@users.noreply.github.com>
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 <116417456+OTheDev@users.noreply.github.com>
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
@rhettinger rhettinger Awaiting requested review from rhettinger
+1 more reviewer
@blaisep blaisep blaisep requested changes
Reviewers whose approvals may not affect merge requirements

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