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