Let plugins customize class MRO #4527
Comments
|
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 It would also help if we can identify other projects that would benefit from this (at at least single well-known project). |
|
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 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? |
|
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 A specific reason I disliked the original pattern is that the provided |
|
As far as real-world usage of
|
|
Also, if you have an idea for how I can type-check my metaclass instances without touching core My primary goal is to get my project typed :) |
The rationale for this MRO hook is documented on python#4527 This patch completely addresses my need for customizing the MRO of types that use my metaclass, and I believe it is simple & general enough for other plugin authors. I did `./runtests.py`, and the only failure is in `testCoberturaParser`, which looks completely unrelated to my changes. Log here: https://gist.github.com/snarkmaster/3f78cf04cfacb7abf2a8da9b95298075 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 -> None hooks 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 fullname often 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 the fullname. For this reason, my base class hook is just return base_class_callback.
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. |
|
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 needsDue 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 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 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 MyPyRegardless 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 proposalIf 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 relevantAs 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. |
|
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. |
gvanrossum
changed the title
The rationale for this MRO hook is documented on python#4527 This patch completely addresses my need for customizing the MRO of types that use my metaclass, and I believe it is simple & general enough for other plugin authors. I did `./runtests.py`, and the only failure is in `testCoberturaParser`, which looks completely unrelated to my changes. Log here: https://gist.github.com/snarkmaster/3f78cf04cfacb7abf2a8da9b95298075 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 -> None hooks 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 fullname often 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 the fullname. For this reason, my base class hook is just return base_class_callback.
|
Thank you!
|
The rationale for this MRO hook is documented on python#4527 This patch completely addresses my need for customizing the MRO of types that use my metaclass, and I believe it is simple & general enough for other plugin authors. I did `./runtests.py`, and the only failure is in `testCoberturaParser`, which looks completely unrelated to my changes. Log here: https://gist.github.com/snarkmaster/3f78cf04cfacb7abf2a8da9b95298075 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 -> None hooks 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 fullname often 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 the fullname. For this reason, my base class hook is just return base_class_callback.
The rationale for this MRO hook is documented on python#4527 This patch completely addresses my need for customizing the MRO of types that use my metaclass, and I believe it is simple & general enough for other plugin authors. I did `./runtests.py`, and the only failure is in `testCoberturaParser`, which looks completely unrelated to my changes. Log here: https://gist.github.com/snarkmaster/3f78cf04cfacb7abf2a8da9b95298075 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 -> None hooks 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 fullname often 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 the fullname. For this reason, my base class hook is just return base_class_callback.
The rationale for this MRO hook is documented on #4527 This patch completely addresses my need for customizing the MRO of types that use my metaclass, and I believe it is simple & general enough for other plugin authors.
|
Fixed by #4567 |
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
mypyplugin 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:
calculate_class_mroto be a member ofSemanticAnalyzerPass2@JukkaL, does this sound like a feature you would take?
Example of needing to customize the MRO:
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).class A(metaclass=M)andclass B(A).M_A_baseandM_B_base. That means thatBactually has both implementation-detail bases.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).M.mro()can simply remove theA-produced base fromB's MRO, and all is well.mypy, removingM_A_basefromB's MRO seems impossible.Cc: @carljm, if you're curious.
The text was updated successfully, but these errors were encountered: