Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upbpo-38524: add note about the implicit and explicit calling of __set_name__ #17364
Conversation
…name__
Hi Florian, thanks for this PR! IMO this section is not the appropriate place in the document to add such a note: This note is about adding descriptors to a class outside of the normal class creation process described in this section. ISTM that "3.3.2.2. Implementing Descriptors" would be the best place for this. Including a reference to "3.3.3.6. Creating the class object" would certainly be appropriate. |
This comment has been minimized.
This comment has been minimized.
Hi Tal, thanks for your reply! I will have a closer look at it and adjust the files accordingly. I guess I should squash the commits together to end up with a single commit? A news entry is not needed here or am I wrong (cause of the failing bedevere/news check)? |
This comment has been minimized.
This comment has been minimized.
Hi Florian, Please don't change the history of commits in a branch with an open PR, i.e. don't rebase, squash etc. It makes understanding the history difficult. We always squash & merge branches when merging, so there's no need for you to squash yourself :) |
This comment has been minimized.
This comment has been minimized.
Okay, I've adjusted the files accordingly. Thanks for the hint with squash and rebase. Didn't find anything about it on devguide. I will have another look at it and if I can't find something like that I will submit issue and PR adding a note about it. After reading the other sections I agree with you, that the note is better placed at the descriptors section. |
This comment has been minimized.
This comment has been minimized.
That would be great! I have worked on that recently as well so please do mention me if you make a PR about this. |
This comment has been minimized.
This comment has been minimized.
From a bit more reading, it would seem also appropriate to suggest setting |
This comment has been minimized.
This comment has been minimized.
This was already added in 2017 (see 3.8. Submitting last paragraph).
In the upcoming days I will check that out, thanks for the hint! I will post my findings here on this PR. Is there anyone else we should mention to review the PR afterwards? |
This comment has been minimized.
This comment has been minimized.
That's not necessary, strictly speaking, as I'm a core dev. Also, this is a documentation PR, and not a major one at that. Let's see what comes up when we dig into |
This comment has been minimized.
This comment has been minimized.
I only found one reference to |
This comment has been minimized.
This comment has been minimized.
From my initial reading, |
This comment has been minimized.
This comment has been minimized.
As I already mentioned, I didn't find anything else about Manually inspecting the objects in the REPL didn't help, either. The example I worked with is listed below as # descriptors.py
class Verbose_attribute():
def __get__(self, obj, type=None) -> object:
return obj.__dict__.get(self.name) or 0
def __set__(self, obj, value) -> None:
obj.__dict__[self.name] = value
def __set_name__(self, owner, name):
self.name = name
class Foo():
attr = Verbose_attribute()
class Bar():
pass
foo = Foo()
print(foo.__class__.__dict__["attr"].__dict__)
bar = Bar()
descr = Verbose_attribute()
Bar.attr = descr
print(bar.__class__.__dict__["attr"].__dict__)
descr.__set_name__(bar, "attr")
print(bar.__class__.__dict__["attr"].__dict__) |
LGTM |
This comment has been minimized.
This comment has been minimized.
Further reading and experimenting hasn't brought up any indication of a need to set |
This comment has been minimized.
This comment has been minimized.
miss-islington
commented
Nov 27, 2019
Thanks @DahlitzFlorian for the PR, and @taleinat for merging it |
…et_name__ (pythonGH-17364) (cherry picked from commit 1bddf89) Co-authored-by: Florian Dahlitz <f2dahlitz@freenet.de>
This comment has been minimized.
This comment has been minimized.
bedevere-bot
commented
Nov 27, 2019
GH-17401 is a backport of this pull request to the 3.8 branch. |
This comment has been minimized.
This comment has been minimized.
bedevere-bot
commented
Nov 27, 2019
GH-17402 is a backport of this pull request to the 3.7 branch. |
…et_name__ (pythonGH-17364) (cherry picked from commit 1bddf89) Co-authored-by: Florian Dahlitz <f2dahlitz@freenet.de>
This comment has been minimized.
This comment has been minimized.
Thanks @taleinat for guiding me! |
…et_name__ (pythonGH-17364)
DahlitzFlorian commentedNov 23, 2019
•
edited
As described in the corresponding issue a note about the implicit and explicit calling of
__set_name__
should be added to the documentation.@taleinat I cannot explicitly request your review. However, I mention you here as you said you'd be happy to review it. Let me know if I missed something.
https://bugs.python.org/issue38524