PEP487: Simpler customization of class creation

[tecki]

BPO	27366
Nosy	@ncoghlan, @berkerpeksag, @tecki, @Vgr255
Files	pep487.patch: The patch for PEP 487 pep487.patch: Changes proposed by Nick pep487.patch: The patch for PEP 487 issue27366_tweaks.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/ncoghlan'
closed_at = <Date 2016-07-31.02:46:45.773>
created_at = <Date 2016-06-22.07:19:27.687>
labels = ['interpreter-core', 'type-feature']
title = 'PEP487: Simpler customization of class creation'
updated_at = <Date 2016-08-20.00:38:51.495>
user = 'https://github.com/tecki'

bugs.python.org fields:

activity = <Date 2016-08-20.00:38:51.495>
actor = 'python-dev'
assignee = 'ncoghlan'
closed = True
closed_date = <Date 2016-07-31.02:46:45.773>
closer = 'ncoghlan'
components = ['Interpreter Core']
creation = <Date 2016-06-22.07:19:27.687>
creator = 'Martin.Teichmann'
dependencies = []
files = ['43835', '43854', '43882', '43946']
hgrepos = []
issue_num = 27366
keywords = ['patch']
message_count = 20.0
messages = ['269050', '269723', '271129', '271132', '271137', '271141', '271163', '271287', '271665', '271667', '271668', '271672', '271673', '271682', '271683', '271717', '271718', '271720', '271721', '273174']
nosy_count = 5.0
nosy_names = ['ncoghlan', 'python-dev', 'berker.peksag', 'Martin.Teichmann', 'abarry']
pr_nums = []
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue27366'
versions = ['Python 3.6']

[tecki]

This is the implementation of PEP-487.

It adds a metaclass to types that calls a method on a class
once it is subclassed. This way one can customize the creation
of classes without the need to write an own metaclass.

As a second functionality, it calls a method on each descriptor
in a class, such that descriptors know their name.

[tecki]

This is a C implementation of PEP-487, directly in the Python core

[ncoghlan]

I started reviewing Martin's patch, and I initially thought I had found a problem with the way __init_subclass__ is currently defined. It turned out I was wrong about it actually being broken, but I *do* now think it's inherently confusing, and we may be able to do something different that's more obviously correct (or at least easier to document - it was proposing revisions to the documentation that got me thinking along this path).

Specifically, I was thinking using super() in either the zero argument form or the explicit form could create an infinite loop due to the way we're currently proposing to interact with the MRO. Consider:

    class BaseClass:
        @classmethod
        def __init_subclass__(cls):
            super(cls, BaseClass).__init_subclass__()

    class SubClass(BaseClass):
        pass

If the initial call made by type.__new__() is effectively "SubClass.mro()[1].__init_subclass__()", then the super() call is going to call BaseClass.__init_subclass__ again.

However, it turned out I was wrong, as that's not what happens: the call made by the type machinery is instead "super(SubClass, SubClass).__init_subclass__", which gets it to the right place in the MRO and causes further super() calls to do the right thing.

However, the "more obviously correct" signature that occurred to me was to do this instead:

    class BaseClass:
        @classmethod
        def __init_subclass__(cls, subcls):
            super(cls, BaseClass).__init_subclass__(subcls)

    class SubClass(BaseClass):
        pass

Then the invocation from type.__new__ could be defined more simply as:

SubClass.mro()[1].__init_subclass__(SubClass)

In all cases then (regardless of where you were in the MRO), "cls" would refer to "the class first in the MRO after the class being defined" and "subcls" would refer to "the class currently being defined".

If you consider the plugin example in the PEP, with the revised signature, it would look like:

    class PluginBase:
        subclasses = []

        def __init_subclass__(cls, subcls, **kwargs):
            super().__init_subclass__(**kwargs)
            cls.subclasses.append(subcls)

And *even if the subclass being defined shadowed the "subclasses" attribute*, this initialisation would still work. (You can still get yourself in trouble if a subclass somewhere else in the MRO shadows the attribute, but that's life in complex type hierarchies)

In the version in the PEP, the fact that "cls" is actually a subclass, and we're relying on the MRO to find "subclasses" is a really subtle implementation detail, while having two parameters makes it clear that "the class defining __init_subclass__" is distinct from the "new subclass being defined".

[ncoghlan]

Scratch that, my revised idea is fundamentally broken, as this example illustrates:

>>> class BaseClass: pass
... 
>>> class OtherClass: pass
... 
>>> class SubClass(OtherClass, BaseClass): pass
... 
>>> SubClass.mro()
[<class '__main__.SubClass'>, <class '__main__.OtherClass'>, <class '__main__.BaseClass'>, <class 'object'>]

The PEP as written handles this correctly, while the alternative signature would fail miserably ("OtherClass" wouldn't chain up to "BaseClass" properly). So I'll review the rest of the patch, and we can figure out the documentation problem later.

[ncoghlan]

Martin's current implementation basically looks good to me - I've mainly suggested some changes to the documentation and additional test cases that help stress test some of the more complex inheritance hierarchies described above, although there are a few other other suggestions as well.

[tecki]

Thanks for the nice review!

I made the proposed changes, see attached patch.

I am still waiting for a decision whether type.__setattr__ should call __set_name__ from python-dev, once that's sorted out I'll implement and test the one or the other behavior.

[gvanrossum]

That's a no from me.

On Sunday, July 24, 2016, Martin Teichmann <report@bugs.python.org> wrote:

Martin Teichmann added the comment:

Thanks for the nice review!

I made the proposed changes, see attached patch.

I am still waiting for a decision whether type.__setattr__ should call
__set_name__ from python-dev, once that's sorted out I'll implement and
test the one or the other behavior.

----------
Added file: http://bugs.python.org/file43854/pep487.patch

---

Python tracker <report@bugs.python.org <javascript:;>>
<http://bugs.python.org/issue27366\>

---

[tecki]

I looked over the patch once more and found some places where
I didn't follow normal coding standards. I changed that, namely
now the code returns -1 meaning an exception happened and 0 on
success, which is what many functions in typeobject.c do.

So I think this patch should be ready.

[ncoghlan]

Martin's latest patch looks good to me, so I'm applying it now and will push it once a local run of the test suite gives it the thumbs up :)

[python-dev]

New changeset ecc7bff738e0 by Nick Coghlan in branch 'default':
Issue bpo-27366: Implement PEP-487
https://hg.python.org/cpython/rev/ecc7bff738e0