On 1 Dec 2017, at 12:29, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 1 December 2017 at 21:04, Ronald Oussoren <ronaldoussoren@mac.com> wrote:
The second question is more a design question: what’s the better design, having __getdescriptor__ as a class method on classes or as method on metaclasses? Either one would work, but a class method appears to be easier to use and with the introduction of __init_subclass__ there is a precedent for going for a class method.
The current PEP claims that a method on a metaclass would be better to avoid subtle problems, but ignores the conceptual cost of adding a metaclass. The subtle problem is that a class can have two direct superclasses with a __getdescriptor__ when using multiple inheritance, but that can already be an issue for other methods and that currently includes __getattribute__ for most of not all usecases where __getdescriptor__ would be useful.
I think it's having it being a method on the metaclass that creates the infinite regress Mark was worried about: since type's metaclass *is* type, if "__getdescriptor__" is looked up as a regular descriptor in its own right, then there's no base case to terminate the recursive lookup.
But type.__getattribute__ is already special, it cannot be the same as its superclass implementation (because that’s object), and object.__getattribute__ logically uses type.__getattribute__ to get at type.__dict__. Adding __getdescriptor__ in the mix makes type.__getattribute__ a bit messier, but not by much.
By contrast, defining it as a class method opens up two options:
1. Truly define it as a class method, and expect implementors to call super().__getdescriptor__() if their own lookup fails. I think this will be problematic and a good way to get the kinds of subtle problems that prompted you to initially opt for the metaclass method.
The only subtle problem is having a class using multiple inheritance that uses two __getdescriptor__ implementations from two superclasses, where both do something beyond looking up the name in __dict__. Failing to call super().__getdescriptor__() is similar to failing to do so for other methods.
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. 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.
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(). The alternative I came up with before writing the PEP is to add a special API that can be used by super(), but that leads to more code duplication as coders would have to implement both __getattribute__ and this other method. I guess another options would be a method that does the work including walking the MRO, but that leads to more boilerplate for users of the API. BTW. I haven’t had a lot of time to work on the implementation. The code in typeobject.c has changed enough that this needs more work than tweaking the patch until it applies cleanly. Ronald