Skip to content

doc: Mention the missing reflected operations #119931

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

Merged

Conversation

paulofreitas
Copy link
Contributor

@paulofreitas paulofreitas commented Jun 2, 2024

Just a minor doc update to maintain consistency with other reflected operations documented in reference/expressions. 👍


📚 Documentation preview 📚: https://cpython-previews--119931.org.readthedocs.build/

To maintain consistency with other arithmetic operations.
@ghost
Copy link

ghost commented Jun 2, 2024

All commit authors signed the Contributor License Agreement.
CLA signed

@bedevere-app bedevere-app bot added docs Documentation in the Doc dir skip news awaiting review labels Jun 2, 2024
@Eclips4 Eclips4 added skip issue needs backport to 3.12 only security fixes needs backport to 3.13 bugs and security fixes labels Jun 2, 2024
Copy link
Member

@Eclips4 Eclips4 left a comment

Choose a reason for hiding this comment

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

Thanks!

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

There is also the same issue for __rpow__, __rtruediv__, __rfloordiv__, __rmod__, __rlshift__, __rrshift__, and maybe others.

It is better to fix them all in a single PR.

Alternatively, we can remove references to __radd__ and __rmul__.

@bedevere-app
Copy link

bedevere-app bot commented Jun 2, 2024

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

Add links to `__rpow__`, `__rtruediv__`, `__rfloordiv__`, `__rmod__`, `__rlshift__` and `__rrshift__`.
@paulofreitas paulofreitas changed the title doc: Mention __rsub__ arithmetic operation doc: Mention the missing reflected operations Jun 2, 2024
@paulofreitas
Copy link
Contributor Author

There is also the same issue for __rpow__, __rtruediv__, __rfloordiv__, __rmod__, __rlshift__, __rrshift__, and maybe others.

It is better to fix them all in a single PR.

Alternatively, we can remove references to __radd__ and __rmul__.

Note that __rand__, __rxor__ and __ror__ were also documented. I went with the first suggestion and added the other missing reflected operations. 👍

Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

For divisions and shifts, I think that it is better to be explicit what dunder customizes what operator.

While we are here, I think it is worth to document also the dunders for the @ operator.

As per review request, improved shifting operators wording and added `@` operators.  👍
@paulofreitas
Copy link
Contributor Author

For divisions and shifts, I think that it is better to be explicit what dunder customizes what operator.

While we are here, I think it is worth to document also the dunders for the @ operator.

Hope I covered both things as intended. 👍

@@ -1314,8 +1318,9 @@ integer; the result is that of mathematical division with the 'floor' function
applied to the result. Division by zero raises the :exc:`ZeroDivisionError`
exception.

This operation can be customized using the special :meth:`~object.__truediv__` and
:meth:`~object.__floordiv__` methods.
This operation can be customized using the special :meth:`~object.__truediv__`,
Copy link
Member

Choose a reason for hiding this comment

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

There are two different operations.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There are two different operations.

Nice catch, I was forgetting this one. 👍

:meth:`~object.__rshift__` methods.
The left shift operation can be customized using the special :meth:`~object.__lshift__`
and :meth:`~object.__rlshift__` methods.

Copy link
Member

Choose a reason for hiding this comment

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

Would not be better to merge these two paragraphs?

Suggested change

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For sure, merged it and did the same formatting for division and floor division operations. 👍

Separates division and floor division and remove spacing from shifting operations.
Copy link
Member

@serhiy-storchaka serhiy-storchaka left a comment

Choose a reason for hiding this comment

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

LGTM.

Thank you for your contribution @paulofreitas.

@serhiy-storchaka serhiy-storchaka enabled auto-merge (squash) June 4, 2024 17:47
@serhiy-storchaka serhiy-storchaka merged commit bf5e106 into python:main Jun 4, 2024
26 of 27 checks passed
@miss-islington-app
Copy link

Thanks @paulofreitas for the PR, and @serhiy-storchaka for merging it 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 4, 2024
…rations (pythonGH-119931)

(cherry picked from commit bf5e106)

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

bedevere-app bot commented Jun 4, 2024

GH-120063 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 4, 2024
miss-islington pushed a commit to miss-islington/cpython that referenced this pull request Jun 4, 2024
…rations (pythonGH-119931)

(cherry picked from commit bf5e106)

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

bedevere-app bot commented Jun 4, 2024

GH-120064 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 4, 2024
@paulofreitas
Copy link
Contributor Author

LGTM.

Thank you for your contribution @paulofreitas.

Thank you for all your help reviewing! Glad to contribute here, looking forward to contribute more! 🎉 🐍

@paulofreitas paulofreitas deleted the rsub-arithmetic-operation branch June 4, 2024 19:12
@serhiy-storchaka
Copy link
Member

Looking forward to review more your PRs.

barneygale pushed a commit to barneygale/cpython that referenced this pull request Jun 5, 2024
kumaraditya303 pushed a commit that referenced this pull request Jun 29, 2024
…ary operations (GH-119931) (#120063)

doc: Mention the missing reflected special methods for all binary operations (GH-119931)
(cherry picked from commit bf5e106)

Co-authored-by: Paulo Freitas <[email protected]>
kumaraditya303 pushed a commit that referenced this pull request Jun 29, 2024
…ary operations (GH-119931) (#120064)

doc: Mention the missing reflected special methods for all binary operations (GH-119931)
(cherry picked from commit bf5e106)

Co-authored-by: Paulo Freitas <[email protected]>
noahbkim pushed a commit to hudson-trading/cpython that referenced this pull request Jul 11, 2024
estyxx pushed a commit to estyxx/cpython that referenced this pull request Jul 17, 2024
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 issue skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants