-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Single axes artist #3835
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Single axes artist #3835
Conversation
👍 Looks like the demo_colorbar_with_inset_locator.py example fails with this change. There is probably need for a modification of axis_grid1 |
I think this resolves #3776 |
I suspect that is has broken a whole bunch of the axes_grid stuff. Don't really understand that error on first pass, my guess is that the base class |
Well, now a different example is broken but for a different reason so that is progress... |
ACCEPTS: an :class:`~matplotlib.axes.Axes` instance | ||
""" | ||
warnings.warn(_get_axes_msg, mplDeprecation, stacklevel=1) | ||
raise Exception("die to find usage") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Still here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, that should not be there...
# call the get method on the parents' property | ||
return Artist.axes.fget(self) | ||
|
||
@axes.setter |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your re-definition of the axes property above looks unnecessary. I think you could omit it, and then replace the @axes.setter
decorator with @Artist.axes.setter
.
The idea of blocking a change in the axes seems good, but I don't have a feel for other implications of the implementation via a switch to properties. What are the other attributes for which you want to abolish the getters and setters? In general, substituting properties seems like a way to make the interface more streamlined and pythonic, but it's a big change in style--although we have snuck in a few properties before. |
I now remember why I was trying to get rid of the getter/setters. They are what are scraped for the docs to generate the ACCEPTS tables. By removing the The other (immediate) candidates for this are the the rest of the things that should not be in that table. |
Failure on 2.6 looks like an unrelated svg font rendering glitch. @tacaswell, is there anything else you want to do on this before it is merged? Maybe squash down to a single commit? |
I think it needs an API_CHANGES entry. |
This is needed because this test tries to add artists from one axes to another which is not really supported. Not 100% that this is fully equivalent to the current test. There is a call to `add_collection` in scatter, but the test might be defeated by some of the other logic in scatter.
Trying to set the axes property before calling up the mro to the base class (eventually Artist) `__init__` which means that the `_axes` attribute is not yet attached to the instance object. - removed setting of `self.axes`, this is taken care of in the baseclass - moved rest of sub-class specific attributes to after call up mro stack
The actual processing and artist creation in Axes.plot happens through the `__call__` method on a _process_plot_var_args instance hanging off the Axes object. The _process_plot_var_args knows what axes it is hanging off of and passed that into the Line2D artists as a kwarg at creation time, which sets the axes of the Line2D. The axes of the Line2D is set again during the `Axes.add_line` method. For the most part this is fine, if un-needed, because the axes is the same objects both time it is set. However when using the mpl_toolkit, Axes objects get wrapped in not-strictly-subclassed objects and the Axes object that the _process_plot_var_args object knows about is not the Axes object that add_line is called on which now results in an exception as it is moving artists between axes.
- deprecate Artist.{get,set}_axes - add axes property - Raise exception when trying to move artists between axes.
5f9cd7d
to
c137a71
Compare
On a silly vanity thing, before I squashed/rebased I had a commit on Nov 28 which would have filled in a gap in what gh thought my longest streak was. I want to keep the three first commits as separate commits because they are each atomic non-backwards (I hope!) changes ( 69dece should win an award for ratio of commit message length to lines changed). Re-ordered into an idealized version of history. |
<sorry for necro'ing this thread>
Thoughts? |
Sounds reasonable to me. Will you put that in an issue or a new pr? Comments on old closed PRs are very likely to be lost |
I'll make a PR. |
This adds check to ensure that you don't add an artist to more than one axes (which does not work because the transform stacks clobber each other).