Skip to content

bpo-8264: Document hasattr and getattr behavior for private attributes#23513

Merged
ethanfurman merged 5 commits intopython:masterfrom
Fidget-Spinner:doc-8264
Feb 2, 2021
Merged

bpo-8264: Document hasattr and getattr behavior for private attributes#23513
ethanfurman merged 5 commits intopython:masterfrom
Fidget-Spinner:doc-8264

Conversation

@Fidget-Spinner
Copy link
Copy Markdown
Member

@Fidget-Spinner Fidget-Spinner commented Nov 25, 2020 •
edited by bedevere-bot
Loading

@Fidget-Spinner @cataliniacob
Convert from diff on bpo
bae9d0c 
Co-Authored-By: Catalin Iacob <iacobcatalin@gmail.com>
@bedevere-bot bedevere-bot added docs Documentation in the Doc dir awaiting review labels Nov 25, 2020
@jstasiak
Copy link
Copy Markdown
Contributor

jstasiak commented Nov 27, 2020

This improves hasattr and setattr documentation but the pull request title says hasattr and getattr – is the mismatch intentional?

@jstasiak
Copy link
Copy Markdown
Contributor

jstasiak commented Nov 27, 2020

To elaborate on that: I think documenting getattr as mentioned in the title would be a nice addition.

@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented Nov 28, 2020 •
edited
Loading

This improves hasattr and setattr documentation but the pull request title says hasattr and getattr – is the mismatch intentional?

No it was a mistake, thanks for catching that. I meant to only document hasatttr and getattr.

To elaborate on that: I think documenting getattr as mentioned in the title would be a nice addition.

Taken from the original patch author: "hasattr is documented in terms of getattr so I would say it's not needed there.". I agree with this.

jstasiak
jstasiak approved these changes Nov 28, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor

@jstasiak jstasiak left a comment

Choose a reason for hiding this comment

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

All right, LGTM.

tiran
tiran reviewed Nov 28, 2020
View reviewed changes
ethanfurman
ethanfurman requested changes Nov 29, 2020
View reviewed changes
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Nov 29, 2020

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.

And if you don't make the requested changes, you will be put in the comfy chair!

@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented Nov 30, 2020

I have made the requested changes; please review again. Thanks.

@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented Nov 30, 2020

Thanks for making the requested changes!

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

@JulienPalard
Copy link
Copy Markdown
Member

JulienPalard commented Dec 1, 2020

LGTM

@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 1, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Jan 1, 2021
@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented Jan 1, 2021 •
edited
Loading

@ethanfurman sorry to disturb, is there anything else I can do to help move this forward?

@github-actions github-actions bot removed the stale Stale PR or inactive for long period of time. label Jan 2, 2021
@github-actions
Copy link
Copy Markdown

github-actions bot commented Feb 2, 2021

This PR is stale because it has been open for 30 days with no activity.

@github-actions github-actions bot added the stale Stale PR or inactive for long period of time. label Feb 2, 2021
@ethanfurman
Copy link
Copy Markdown
Member

ethanfurman commented Feb 2, 2021

My apologies, I lost track of this. Reviewing now.

@ethanfurman ethanfurman merged commit 2edaf6a into python:master Feb 2, 2021
@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented Feb 2, 2021

My apologies, I lost track of this. Reviewing now.

No worries. Thanks for the review :)!

adorilson pushed a commit to adorilson/cpython that referenced this pull request Mar 13, 2021
@Fidget-Spinner @cataliniacob @adorilson
bpo-8264: Document hasattr and getattr behavior for private attributes (
92dc5e9 
pythonGH-23513)

Clarify ``getattr`` and ``setattr`` requirements for accessing name-mangled attributes

Co-Authored-By: Catalin Iacob <iacobcatalin@gmail.com>
@Fidget-Spinner Fidget-Spinner deleted the doc-8264 branch May 16, 2021 07:49
@miss-islington
Copy link
Copy Markdown
Contributor

miss-islington commented May 19, 2021

Thanks @Fidget-Spinner for the PR, and @ethanfurman for merging it 🌮🎉.. I'm working now to backport this PR to: 3.9.
🐍🍒⛏🤖

miss-islington pushed a commit to miss-islington/cpython that referenced this pull request May 19, 2021
@Fidget-Spinner @miss-islington
bpo-8264: Document hasattr and getattr behavior for private attributes (
eb105c9 
pythonGH-23513)

Clarify ``getattr`` and ``setattr`` requirements for accessing name-mangled attributes

Co-Authored-By: Catalin Iacob <iacobcatalin@gmail.com>
(cherry picked from commit 2edaf6a)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
@bedevere-bot
Copy link
Copy Markdown

bedevere-bot commented May 19, 2021

GH-26238 is a backport of this pull request to the 3.9 branch.

@iritkatriel
Copy link
Copy Markdown
Member

iritkatriel commented May 21, 2021

Does this need a backport to 3.10 as well?

@Fidget-Spinner
Copy link
Copy Markdown
Member Author

Fidget-Spinner commented May 22, 2021 •
edited
Loading

Does this need a backport to 3.10 as well?

Nope. It was merged back when main was 3.10. Thanks for merging the backport!

Edit: Oops I mistook this PR for the backport. Please ignore the second part. The backport to 3.9 hasn't landed yet.

iritkatriel pushed a commit that referenced this pull request May 22, 2021
@miss-islington @Fidget-Spinner
bpo-8264: Document hasattr and getattr behavior for private attributes (
11b5045 
GH-23513) (GH-26238)

Clarify ``getattr`` and ``setattr`` requirements for accessing name-mangled attributes

Co-Authored-By: Catalin Iacob <iacobcatalin@gmail.com>
(cherry picked from commit 2edaf6a)

Co-authored-by: Ken Jin <28750310+Fidget-Spinner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@tiran tiran tiran left review comments

@ethanfurman ethanfurman ethanfurman approved these changes

+1 more reviewer

@jstasiak jstasiak jstasiak approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

docs Documentation in the Doc dir skip news stale Stale PR or inactive for long period of time.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

9 participants

@Fidget-Spinner @jstasiak @bedevere-bot @JulienPalard @ethanfurman @miss-islington @iritkatriel @tiran @the-knights-who-say-ni