On 2 December 2017 at 01:12, Ronald Oussoren <ronaldoussoren@mac.com> wrote:
On 1 Dec 2017, at 12:29, Nick Coghlan <ncoghlan@gmail.com> wrote:
2. Define it as a class method, but have the convention be for the *caller* to worry about walking the MRO, and hence advise class implementors to *never* call super() from __getdescriptor__ implementations (since doing so would nest MRO walks, and hence inevitably have weird outcomes). Emphasise this convention by passing the current base class from the MRO as the second argument to the method.
But that’s how I already define the method, that is the PEP proposes to change the MRO walking loop to:
for cls in mro_list: try: return cls.__getdescriptor__(name) # was cls.__dict__[name] except AttributeError: # was KeyError pass
Note that classes on the MRO control how to try to fetch the name at that level. The code is the same for __getdescriptor__ as a classmethod and as a method on the metaclass.
That's not exactly the same as what I'm suggesting, and it's the part that has Mark concerned about an infinite regression due to the "cls.__getdescriptor__" subexpression. What I'm suggesting: try: getdesc = base.__dict__["__getdescriptor__"] except KeyError: # Use existing logic else: try: getdesc(cls, base, name) except AttributeError: pass * Neither type nor object implement __getdescriptor__ * Calling super() in __getdescriptor__ would be actively discouraged without a base class to define the cooperation rules * If it's missing in the base class dict, fall back to checking the base class dict directly for the requested attribute * cls is injected into the call by the MRO walking code *not* the normal bound method machinery * Only "base.__dict__" needs to be assured of getting a hit on every base class What's currently in the PEP doesn't clearly define how it thinks "cls.__getdescriptor__" should work without getting itself into an infinite loop.
I don’t think there’s a good technical reason to pick either option, other than that the metaclass option forces an exception when creating a class that inherits (using multiple inheritance) from two classes that have a custom __getdescriptor__. I’m not convinced that this is a good enough reason to go for the metaclass option.
I'm still not clear on how you're planning to break the recursive loop for "cls.__getdescriptor__" when using a metaclass.
The reason I'm liking option 2 is that it leaves the existing __getattribute__ implementations fully in charge of the MRO walk, and *only* offers a way to override the "base.__dict__[name]" part with a call to "base.__dict__['__getdescriptor__'](cls, base, name)" instead.
Right. That’s why I propose __getdescriptor__ in the first place. This allows Python coders to do extra work (or different work) to fetch an attribute of a specific class in the MRO and works both with regular attribute lookup as well as lookup through super().
Yeah, I'm definitely in agreement with the intent of the PEP. I'm just thinking that we should aim to keep the expected semantics of the hook as close as we can to the semantics of the code it's replacing, rather than risking the introduction of potentially nested MRO walks (we can't outright *prevent* the latter, but we *can* quite clearly say "That's almost certainly a bad idea, avoid it if you possibly can"). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia