-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Deprecate LocatableAxes from toolkits #10403
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
Conversation
9743d47
to
e994a5f
Compare
I would rather push this to 3.0 (hoping to tag 2.2RC1 this weekend....only a week late) |
I haven't looked in detail, but I will be glad to see this go in whenever it does, and the duplication go away. |
e994a5f
to
2805e5c
Compare
Updated deprecation messages to say 3.0 instead of 2.2. |
2805e5c
to
6dfa3dd
Compare
Rebased due to conflicts. These files are messy, so it would be nice to get this in quicker to reduce conflicts. |
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.
This seems low-risk; let's get it in.
@anntzer or @dopplershift, perhaps one of you could have a look and merge this if it looks OK, so it doesn't have to be rebased again. |
I'm not even close to knowing enough about axes_locator to decide whether the functionality is indeed duplicated or not. |
Mainly you can look at the |
if 'sharex' in kwargs and 'sharey' in kwargs: | ||
raise ValueError("Twinned Axes may share only one axis.") | ||
ax2 = type(self)(self.figure, self.get_position(True), *kl, **kwargs) | ||
ax2.set_axes_locator(self.get_axes_locator()) |
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.
The base Axes._make_twin_axes
is missing this locator-copying line--maybe that is a bug.
It looks to me like we need to either support I'm not sure which would be best; it's a little odd to have an attribute in |
There are times when @leejjoon recommended using |
Ooh, this fell through the cracks. Any chance we can resurrect? |
I can rebase; depends on @efiring's thoughts. |
My opinion is highly biased as I am the one who introduced the axes_locator. But I would like to see it stays. While I understand this is underused, this makes certain things a lot easier (although may not relevant for normal users). Also, #11026 requires axes_locator. |
This doesn't remove it though; it deprecates the one in axes_grid for the one in the main library, which appears to be what #11026 uses. |
Yes. I was just expressing my opinion to what @efiring said, and I was referring to axes_locator in the base. I am happy with the axes_grid becoming deprecated. |
I'm more than a little confused--I haven't looked at this for a while, it deals with functionality I am not familiar with, and so the spinup in trying to come back to it is considerable. @leejjoon, it's good to hear from you--but could you be more explicit with respect to this PR? Do you favor it as is (after a rebase), or with minor changes, or major changes, or not at all? |
I am in favor of this PR. In fact, I think we can actually remove |
6dfa3dd
to
36e3701
Compare
This is provided for backwards-compatibility, so point directly to the implementations instead of the compatibility location.
All its functionality is provided by the matplotlib.axes.Axes class now so it does not need to exist as all alternative Axes classes derive from the main one.
It's similar to LocatableAxesBase and provided in base Axes classes.
It's now no longer used for anything, since maxes.Axes is locatable already.
36e3701
to
7421b0e
Compare
OK, rebase is done. |
PR Summary
LocatableAxes
in the toolkits provides stuff likeget_axes_locator
, but these are provided in the baseAxes
class. So deprecate all of the classes and related functions. Additionally, remove all code from the deprecatedaxes_grid
, and point it to the split apart implementations.PR Checklist