Repository navigation
Let plugins customize class MRO #4527
Description
The Python metaclass data model allows the mro() method to be overloaded. This is a genuinely useful feature for dealing with metaclasses whose classes must support inheritance. See "Example" below.
I wrote a mypy plugin for type-checking instances of my metaclass. I was able to model its semantics almost correctly with the existing plugin hooks, except for one thing — customizing the MRO.
The best way I can get my type-checker unblocked is by adding a plugin hook for customizing the MRO. My idea is roughly this:
- Move
calculate_class_mroto be a member ofSemanticAnalyzerPass2 - Add a plugin hook that gets to customize the computed MRO.
@JukkaL, does this sound like a feature you would take?
Example of needing to customize the MRO:
- My metaclass
Minjects a implementation-detail baseM_classname_baseinto each class it creates.M's semantics demand thatM_classname_basebe at the very end of the MRO (to allow overloading of its features). - Let us define
class A(metaclass=M)andclass B(A). - Each class injects its own base:
M_A_baseandM_B_base. That means thatBactually has both implementation-detail bases. - Unfortunately,
M_A_baseandM_B_baseconflict, andM_B_baseends up afterM_A_basein the MRO, which is the opposite of what you want (child must overload parent, not vice versa). - At runtime,
M.mro()can simply remove theA-produced base fromB's MRO, and all is well. - In current
mypy, removingM_A_basefromB's MRO seems impossible.
Cc: @carljm, if you're curious.
Activity
JukkaL commentedon Feb 1, 2018
At first this sounds like a pretty simple change, but there are some edge cases:
There probably are more things that could go wrong. Maybe you could go through all
TypeInfoattributes and analyze which of them might be stale after an MRO change? Maybe we should reject certain MRO changes if they are hard to support?It would also help if we can identify other projects that would benefit from this (at at least single well-known project).
snarkmaster commentedon Feb 1, 2018
Here's a proof-of-concept patch: snarkmaster@63f4aa4
I think your concerns are totally reasonable. Changing the MRO is "very advanced use-case" territory, and you can really break the semantics of your classes.
However, let's assume for a moment that the author of the metaclass knows what they are doing — if not, their runtime ends up utterly broken anyway. As I see it, the job of
mypyis to allow statically modeling runtime semantics using the parsed AST.At the moment, all the other plugin hooks are 'at your own risk' — I can completely rewrite a class or its bases from the metaclass or base class hook.
As such, I think it's not too bad to give plugin authors a very simple hook to emulate a well-established part of Python's data model. See the docs here:
https://docs.python.org/3/library/stdtypes.html?highlight=mro#class.mro
What do you think about the simple patch?
snarkmaster commentedon Feb 1, 2018
PS You will notice in the patch that I deviated from the pattern of "some_hook(fullname) returns a callback, which takes a context". I did this as a suggestion for improvement. At the very least, all of the
-> Nonehooks could be simplified in this fashion (and probably the others, too). I'm happy to put up a patch for that, if there's a process for dealing with a breaking change in the plugin API.A specific reason I disliked the original pattern is that the provided
fullnameoften does not have enough information for the hook to decide whether it cares, so the hook has to return a callback always. And yet, we spend cycles specifically extracting & passing just thefullname. For this reason, my base class hook is justreturn base_class_callback.snarkmaster commentedon Feb 1, 2018
As far as real-world usage of
def mro, I have not yet found a big public project that heavily relies on it. But, it's not an unpopular idea:mro()-enabled metaclass to provide a backwards-compatibility shim: https://github.com/zopefoundation/ExtensionClass/blob/825fbf48a01eaf70d273ca9e9175f7465b38ba14/src/ExtensionClass/__init__.pymro()-enabled metaclass to paper over a design bug in Django: http://www.memonic.com/user/danilo/folder/django/id/1xmoosnarkmaster commentedon Feb 1, 2018
Also, if you have an idea for how I can type-check my metaclass instances without touching core
mypy, I'd love to hear it. The "Example" story in the issue description is complete and accurate.My primary goal is to get my project typed :)
Add a `customize_class_mro` plugin hook
gvanrossum commentedon Aug 1, 2018
And yet you don't appear to link to your project, so your concrete needs are pretty vague -- you gave an elaborate example, but it's not code, and I don't follow every part of it. In particular "At runtime, M.mro() can simply remove the A-produced base from B's MRO, and all is well" sounds mysterious -- who is calling mro() here?
You didn't help your case by combining it with a separate API change proposal.
Given that it's now August, maybe you no longer care, and then I would respectfully propose to just close this issue and the PR.
snarkmaster commentedon Aug 2, 2018
Point of order: I think your comment would have read better without "And yet" and without "You didn't help your case". Factual and "I" statements cause less friction.
Project & specific needs
Due to the silence on this task, I put the typing work on hold.
What is the project-specific reason?
The MyPy-related code in question is not committed anywhere public, because I put it on hold. The code that is public has nothing to do with my request. I built a working prototype in a private branch, but for the purposes of the discussion you probably don't want to be reviewing 1000s of lines of code when 10s will suffice.
In words, the piece of code that needs the MyPy hook is a loose analog of a
NamedTupleor a PyrsistentPRecordor a frozendataclass. My thing supports C3 inheritance for composing fields. Let's not debate too heatedly whether that's a good idea, given that it seems to serve well in at least my current codebase :)My reason for MRO customization is as follows. The core implementation of these "frozen classes" is supplied by an implementation base class, which the metaclass injects on each of the "frozen classes" in the hierarchy. But, only the last-in-the-MRO implementation is needed, so I overload
mro()to discard the earlier ones. Thus, I also need to hide those implementation bases from MyPy, since they would otherwise break type inference and creating spurious errors.If you're willing to discuss adding an MRO customization hook to MyPy, then I will extract a minimal code example of how I am doing MRO customization. Let me know.
Request for MyPy
Regardless of the specific reason, at the end of the day, MyPy currently assumes that the MRO of a class is as-declared. And that's not the case with my metaclass. A hook of this sort in MyPy would be necessary and sufficient.
Separate API change proposal
If that drive-by suggestion is annoying to you, I'm happy to stick with the current API. I saw an opportunity for improvement and pointed it out. Do with it what you will.
This is still relevant
As you well know, type-checks are never "urgent" or "mandatory". That said, I would love to have working type-checks.
If you confirm the concrete list of changes you want to see on the PR, I will gladly update it.
gvanrossum commentedon Aug 2, 2018
Fine. Submit the minimal PR that follows the established style and allows you to solve your problem. Someone will review it. Understand that the plugin API is not frozen or documented and can (and will!) change without notice.
[-]Would you take a patch to let plugins customize class MRO?[/-][+]Let plugins customize class MRO[/+]Add a `customize_class_mro` plugin hook
snarkmaster commentedon Aug 6, 2018
Thank you!
customize_class_mroplugin hook #4567. Specifically:customize_class_mroto the more complex local convention ofget_customize_class_mro_hook. If you see merit in my simplification, I can open a separate PR for it.masterofpython/mypyand resolved conflicts.Add a `customize_class_mro` plugin hook
Add a `customize_class_mro` plugin hook
Add a `customize_class_mro` plugin hook (#4567)
ilevkivskyi commentedon Sep 3, 2018
Fixed by #4567