-
-
Notifications
You must be signed in to change notification settings - Fork 30.2k
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-60712: Include the "object" type in the lists of documented types #103036
base: main
Are you sure you want to change the base?
Conversation
The original author was Martin Panter, @vadmium. I fixed the co-authorship note. |
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.
Standard reminder: You can directly apply all the suggestions you want in one go by going to Files changed
-> Clicking Add to batch on each suggestion -> When done, clicking Commit
Thanks! Various further textual, syntactic and formatting fixes.
Misc/NEWS.d/next/Documentation/2023-03-28-22-24-45.gh-issue-60712.So5uad.rst
Outdated
Show resolved
Hide resolved
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
Co-authored-by: C.A.M. Gerlach <[email protected]>
'dictionaries' to 'dict' Co-authored-by: C.A.M. Gerlach <[email protected]>
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.
I have not reviewed changes to special methods section. Will make other changes on my local copy.
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 |
Co-authored-by: Éric <[email protected]>
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.
Thanks for the changes to the standard library docs and the additional test.
I recommend that several of the additions to the Language Reference should be removed. For the language reference, we are trying to be concise and focus on syntax and semantics of the language with less focus on the implementation details (which would be appropriate in the standard library docs)..
@@ -1733,10 +1741,10 @@ Basic customization | |||
|
|||
.. method:: object.__str__(self) |
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.
I thought we had a doc bug years ago where we came up with a convention to differentiate object.__str__
(actual method that exists in the object
class), anyclass.__bases__
(method that exists on all class; explaining the data model) and someclass.__str__
(method you can define in your class to implement a protocol)
I guess we didn’t…
…tamodel.rst for improved clarity.
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.
Thanks @furkanonder.
You are welcome. |
Co-authored-by: Éric <[email protected]>
:class:`tuples <tuple>`) or :term:`mappings <mapping>` (like | ||
:class:`dictionaries <dict>`), | ||
:term:`dictionaries <dictionary>`), |
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.
🤔 Hm… on the line before, we see a general term used (with a link to glossary) and a concrete example in parens (list and tuple links), then we have the general/glossary term mappings and should keep the specific type dict in parens to keep the intent of the text. On the other hand, if the glossary term for dictionaries is a better explanation than the dict type/function doc, and more specific than the term for mappings, the changed link could be an improvement.
User-defined classes (and the :class:`object` class itself) have :meth:`__eq__` | ||
and :meth:`__hash__` methods by default; with them, all objects compare |
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.
Is this saying:
User-defined classes (and the :class:`object` class itself) have :meth:`__eq__` | |
and :meth:`__hash__` methods by default; with them, all objects compare | |
User-defined classes have :meth:`__eq__` and :meth:`__hash__` methods | |
by default (inherited from the :class:`object` class); with them, all objects compare |
Co-Authored-By: @vadmium