You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Replace all the metaclasses possible with __init_ #7144
PEP 487, implemented in py 3.6, introduced a useful feature: the __init_subclass__ method. It's basically a hook that lets you do custom things at class creation time. As the PEP points out, a lot of fairly complicated metaclass code amounts to doing just this.
So the idea for this issue is to try to replace usage of metaclasses in astropy wherever possible with __init_subclass__. In general metaclasses are a confusing concept, so this is potentially a big readability win. Two outstanding questions, though:
How much of the metaclass use in astropy will this be useful for? I think (almost?) all the metaclass usage in coordinates would be replaceable by this, and that would make it a lot more readable. By contrast, some of the modeling usage certainly requires trued metaclasses. So some investigative work is called for...
Using __init_subclass__ nominally requires py 3.6, which would man we'd have to wait for Astropy 3.3, which is when 3.6+ is required. but I found https://gist.github.com/pjeby/75ca26f8d2a7a0c68e30 , which we could put in astropy.utils.compat along the lines of what I've given below, which I think should make it all seamlessly transition between py 3.6 and earlier?:
if sys.version_info.major > 3 or (sys.version_info.major == 3 and sys.version_info.minor >= 6):
InitSubclassBase = object
else:
... pep_487.py from the aforementioned gist ...
InitSubclassBase = init_subclasses # (from the gist code)
and then in the cases where the metaclass are replaced they just need to subclass from InitSubclassBase
And @Cadair reminds me to point out that this is a "meta-issue" about removing metaclasses. 😮
EDIT (by @mhvk): should really do a full search, but probably at least good to keep track of what has been done and what is outstanding but plausible (latter likely incomplete):
The TimeFormatMeta class is certainly a good candidate for this. I had forgotten about this new feature but it is very sensible and useful.
The one thing I'd say is that in the example of TimeFormatMeta there is really very little "win" to doing this now for 3.1-dev instead of waiting a year for 3.3-dev. The actual code would be fairly identical, and there is really no issue with e.g. multiple meta-class inheritance etc.
So I think it would be worth first identifying a real driver for putting in the compatibility layer now and then backing it out later, instead of updating code just once after 3.2 is released around Apr, 2019.
Yes, @embray mentioned this mid-2016 on astropy-dev - https://groups.google.com/d/topic/astropy-dev/gpRTvftTe_E/discussion - it definitely would be handy for representations, but probably not for units. I can see the benefits of both experimenting with it and waiting to 3.6 is our minimum version. So perhaps it should just be whether someone is sufficiently excited to go ahead?
In the meantime, the itemized list above for changing metaclasses to __init_subclass__ is nearly done. I had a quick look at io.ascii, but there the metaclass is used by both the standard and the fast reader, so it seemed one would have to replace the metaclass with a separate base class, which perhaps is less useful. I also had a quick look at other uses, but nothing seemed as obvious a case as the ones we have done already. So, my sense would be to close this issue once io.fits is merged.
Activity
taldcroft commentedon Feb 1, 2018
The
TimeFormatMetaclass is certainly a good candidate for this. I had forgotten about this new feature but it is very sensible and useful.The one thing I'd say is that in the example of
TimeFormatMetathere is really very little "win" to doing this now for 3.1-dev instead of waiting a year for 3.3-dev. The actual code would be fairly identical, and there is really no issue with e.g. multiple meta-class inheritance etc.So I think it would be worth first identifying a real driver for putting in the compatibility layer now and then backing it out later, instead of updating code just once after 3.2 is released around Apr, 2019.
mhvk commentedon Feb 1, 2018
Yes, @embray mentioned this mid-2016 on astropy-dev - https://groups.google.com/d/topic/astropy-dev/gpRTvftTe_E/discussion - it definitely would be handy for representations, but probably not for units. I can see the benefits of both experimenting with it and waiting to 3.6 is our minimum version. So perhaps it should just be whether someone is sufficiently excited to go ahead?
mhvk commentedon Aug 25, 2021
In the meantime, the itemized list above for changing metaclasses to
__init_subclass__is nearly done. I had a quick look atio.ascii, but there the metaclass is used by both the standard and the fast reader, so it seemed one would have to replace the metaclass with a separate base class, which perhaps is less useful. I also had a quick look at other uses, but nothing seemed as obvious a case as the ones we have done already. So, my sense would be to close this issue onceio.fitsis merged.saimn commentedon Sep 1, 2021
Closing as suggested by Marten.