FIX: if we have already subclassed mixin class just return #20403
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
If subplot_class_factory is passed a base class that already sub-classes the mixin class, simply return the base class. This will allow third-party packages to derive from maxes.Subplot rather than maxes.Axes which will allow easier (by ensuring that our classes and the third-party classes do not mix in the mro).
anntzer
approved these changes
Jun 10, 2021
jklymak
approved these changes
Jun 10, 2021
|
Feel free to self merge. I'm not sure if this needs a back port... |
|
I would like to backport this to give proplot an easy path to un-breaking with the 3.4 series. |
|
Owee, I'm MrMeeseeks, Look at me. There seem to be a conflict, please backport manually. Here are approximate instructions:
And apply the correct labels and milestones. Congratulation you did some good work ! Hopefully your backport PR will be tested by the continuous integration and merged soon! If these instruction are inaccurate, feel free to suggest an improvement. |
tacaswell
added a commit
to tacaswell/matplotlib
that referenced
this pull request
Jun 10, 2021
…n class just return Merge pull request matplotlib#20403 from tacaswell/noop_subplot_class_factory FIX: if we have already subclassed mixin class just return Conflicts: lib/matplotlib/cbook/__init__.py - logic is in axes/_subplots.py on this branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR Summary
If subplot_class_factory is passed a base class that already sub-classes
the mixin class, simply return the base class.
This will allow third-party packages to derive from maxes.Subplot rather
than maxes.Axes which will allow easier (by ensuring that our classes
and the third-party classes do not mix in the mro).
This allows a fix to proplot-dev/proplot#248 by changing which mpl class they inherit from (which brings them closer inline with the mpl36 goal of merging the subplot logic fulling into Axes
Currently we have:
The key detail is that in the first case the resoltion order bounces through mpl -> proplot -> mpl which means the code above / below
supre().__init__(...)calls gets interleaved between the two code bases. This means that when the call tosuper().__init__()returns in the proplot code they can not assume that the Axes is fully initialized (because any code that runs after the super() on the mpl side may will not have run yet becaues it will run after the proplot__init__returns). In the second case all of the proplot classes are before all of our classes so that if proplot doessuper().__init__(...)they can assume that the object is in a, from mpl's point of view, fully initialized and consistent state.PR Checklist
pytestpasses).flake8on changed files to check).flake8-docstringsand runflake8 --docstring-convention=all).doc/users/next_whats_new/(follow instructions in README.rst there).doc/api/next_api_changes/(follow instructions in README.rst there).