bpo-38524: add note about the implicit and explicit calling of __set_name__

[DahlitzFlorian]

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

[taleinat]

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.

[DahlitzFlorian]

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)?

[taleinat]

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 :)

[DahlitzFlorian]

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.

[taleinat]

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.

That would be great! I have worked on that recently as well so please do mention me if you make a PR about this.

[taleinat]

From a bit more reading, it would seem also appropriate to suggest setting __objclass__ in similar cases as those where __set_name__ would need to be called explicitly. I'm not actually familiar with it and haven't the time right now to dig deeper, but it's worth checking out.

[DahlitzFlorian]

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 was already added in 2017 (see 3.8. Submitting last paragraph).

From a bit more reading, it would seem also appropriate to suggest setting __objclass__ in similar cases as those where __set_name__ would need to be called explicitly. I'm not actually familiar with it and haven't the time right now to dig deeper, but it's worth checking out.

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?

[taleinat]

@DahlitzFlorian

Is there anyone else we should mention to review the PR afterwards?

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 __objattr__, and if we're unsure we can ping Serhiy and Nick who commented on the bpo issue.

[DahlitzFlorian]

From a bit more reading, it would seem also appropriate to suggest setting __objclass__ in similar cases as those where __set_name__ would need to be called explicitly. I'm not actually familiar with it and haven't the time right now to dig deeper, but it's worth checking out.

I only found one reference to __objclass__ in the documentation (right below the inserted note) and can't see why we should include a note about setting it, too. Perhaps someone else has more knowledge about it. The references inside the source code didn't enlighten me, either.

[taleinat]

I only found one reference to __objclass__ in the documentation (right below the inserted note) and can't see why we should include a note about setting it, too.

From my initial reading, __objclass__ appears to be another thing one should explicitly set on descriptors added to a class after class initialization. It is expected to be there by certain tools, e.g. in the inspect and pydoc modules. If my understanding is correct, then it seems to make sense to mention it alongside __set_name__.

[DahlitzFlorian]

As I already mentioned, I didn't find anything else about __objclass__ in the documentation than the paragraph after the note inserted by this PR. That's why I searched in the PEPs to find some more information about it and found it in PEP 252, PEP 575 and PEP 580. At least two of the helped me understand the purpose of __objclass__ better. However, I wasn't able to actually access __objclass__ anywhere. The paragraph below the note mentions, that the inspect module uses it. However, the inspect module on its own doesn't mention it in its documentation.

Manually inspecting the objects in the REPL didn't help, either. The example I worked with is listed below as descriptor.py. As I can't find an obvious usage, don't find anything about it in the corresponding documentations and wasn't able to access it in the REPL (deep deep down), I guess we don't need to mention it in the note. Nonetheless, I would be glad if someone else finds anything to support my thesis and merge the PR or correct it, so we can add it to the note and merge afterwards.

# 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__)

[taleinat]

LGTM

[taleinat]

Further reading and experimenting hasn't brought up any indication of a need to set __objclass__, so we should indeed leave that alone. Thanks for digging into that @DahlitzFlorian!

[miss-islington]

Thanks @DahlitzFlorian for the PR, and @taleinat for merging it 🌮🎉.. I'm working now to backport this PR to: 3.7, 3.8.
🐍🍒⛏🤖 I'm not a witch! I'm not a witch!

[bedevere-bot]

GH-17401 is a backport of this pull request to the 3.8 branch.

[bedevere-bot]

GH-17402 is a backport of this pull request to the 3.7 branch.

[DahlitzFlorian]

Thanks @taleinat for guiding me!