Skip to content

bpo-38255: Replace "method" with "attribute" in the description of super() #16331

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

Closed
wants to merge 5 commits into from

Conversation

geryogam
Copy link
Contributor

@geryogam geryogam commented Sep 23, 2019

This PR will make the following change to the Built-in Functions chapter of the library documentation:

  • replace the word "method" with the more general word "attribute" in the description of super(), since super() is not restricted to method access but can also do data attribute access:
>>> class A:
...     x = True
... 
>>> class B(A):
...     x = False
... 
>>> B().x
False
>>> super(B, B()).x
True

https://bugs.python.org/issue38255

@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Sep 23, 2019
@geryogam geryogam changed the title Update functions.rst Replace "method" with "attribute" in the description of super() Sep 23, 2019
@geryogam geryogam changed the title Replace "method" with "attribute" in the description of super() bpo-38255: Replace "method" with "attribute" in the description of super() Sep 23, 2019
@rhettinger rhettinger self-assigned this Sep 23, 2019
Copy link
Contributor

@rhettinger rhettinger left a comment

Choose a reason for hiding this comment

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

I don't want to do this. Delegating methods is the primary use case for super(), perhaps by a factor of over a hundred to one.

We can add "(or attributes)" but should keep the wording focused on methods.

@bedevere-bot
Copy link

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.

@geryogam
Copy link
Contributor Author

geryogam commented Sep 23, 2019

Alright @rhettinger. I have used "(or data attributes)" instead of merely "(or attributes)" as a method is also an attribute (since the official tutorial uses this nomenclature: "There are two kinds of valid attribute names, data attributes and methods.")

I have made the requested changes; please review again.

@bedevere-bot
Copy link

Thanks for making the requested changes!

@rhettinger: please review the changes made to this pull request.

@bedevere-bot
Copy link

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.

@rhettinger
Copy link
Contributor

See the alternative PR at #16368 which adds a sentence to the section on the various ways to use super().

When people are first taught about super() (something I do frequently), it isn't a good idea to lead-off the discussion with attribute access. If user's first experiment is with attribute access, their odds of success will be low and it will distract from learning the primary, intended use of the tool.

Copy link
Contributor

@rhettinger rhettinger left a comment

Choose a reason for hiding this comment

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

Let's not merge this until the alternative PR has been discussed.

@willingc
Copy link
Contributor

Thanks @maggyero for this PR and your efforts on documentation.

After reading this PR and #16368, I believe #16368 covers the most common usage and mentions the possibility of attributes as well. I recommend merging #16368 since I believe it gives more clarity to the reader.

Copy link
Contributor

@willingc willingc left a comment

Choose a reason for hiding this comment

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

I recommend merging #16368 and closing this PR. Thanks all.

@geryogam
Copy link
Contributor Author

geryogam commented Sep 25, 2019

Thanks @rhettinger for the review and the alternative PR, it makes sense. I am closing this one.

@geryogam geryogam closed this Sep 25, 2019
@geryogam geryogam deleted the patch-4 branch September 25, 2019 12:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting changes docs Documentation in the Doc dir skip news
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants