Skip to content

Rewrite the logic of formatters for getting the tick locations #13438

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

Open
timhoffm opened this issue Feb 14, 2019 · 13 comments
Open

Rewrite the logic of formatters for getting the tick locations #13438

timhoffm opened this issue Feb 14, 2019 · 13 comments
Labels
keep Items to be ignored by the “Stale” Github Action status: inactive Marked by the “Stale” Github Action topic: ticks axis labels

Comments

@timhoffm
Copy link
Member

Follow up from discussions in #13323. This is essentially a placeholder so that the ideas from there do not get lost.

The target state is described in #13323 (comment) and following comments.

@jklymak
Copy link
Member

jklymak commented Nov 25, 2020

Reproducible issue:

Summary

Locators and Formatters stash info about what axis they are attached to and often use that info when they are called (via locator()). This makes locators (and some formatters) un-usable across multiple axes.

Code for reproduction

import matplotlib.pyplot as plt
import matplotlib as mpl
fig, ax = plt.subplots()
loc = mpl.ticker.AutoLocator()
ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(loc)
ax.set_ylim([0.6, 0.8])
plt.show()

Actual outcome

Figure_1

Expected outcome

Probably the same outcome as

loc = mpl.ticker.AutoLocator()
ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(mpl.ticker.AutoLocator())

FigureExpected

@jklymak
Copy link
Member

jklymak commented Nov 25, 2020

It seems that there are a few strategies, some of which have been tried:

Lock out adding a formatter to a second axes

Pros: Easy to implement. Unambiguous to the user - just throws an error if they try and add to a second axis.

Cons: Not backwards compatible. Users cannot modify just one locator or formatter and have it apply to more than one axis.

Copy locator when assigned to an axis:

Pros: relatively easy

Cons: user doesn't know about this copy and will not be able to operate on the Locator instance they created.

modify locator() to accept axis argument

i.e. ticks = self.major_locator(axis=self)

Pros: this would allow explicitly updating the tick creation when needed.

Cons: third party locators would not have this kwarg, so we could not call this unconditionally. However, we could have a try-except clause that deprecates locators with no axis, and then falls back to the stashed axis.

@jklymak jklymak added this to the v3.4.0 milestone Nov 25, 2020
@tacaswell
Copy link
Member

I think "make a copy" is off the table as I suspect that will cause far greater confusion. I also disagree that it is obviously easy to implement. The Locators can be arbitrary Python objects so being able to reliably making a fully independent copy is not a given.

I do like option 3 as it is slowly pulling back distributed shared state.

I think we need to ask does "having the same locator (axis1.get_major_locator() is ais2.get_major_locator()) mean "we use the same locations on both axis" or "we use the same method to determine where the locations are on both axis". Currently it means the first (and we break the symmetry as to which axis is the lead by the last one it is attached to). If we change to "using the same method" as the meaning, then how can we get back the ability to have "the same tick loctations" on more than one axis? I guess we could add a MirrorLocactor(other_axis) that explicitly implements that.

@jklymak
Copy link
Member

jklymak commented Nov 25, 2020

I think we need to ask does "having the same locator (axis1.get_major_locator() is ais2.get_major_locator()) mean "we use the same locations on both axis" or "we use the same method to determine where the locations are on both axis".

Hmm I hadn't considered that. Is there call for the first case if the axis limits are different? Right now we have shared axes, and the limits are held the same, so if they have the same method they will get the same ticks. I guess if for some reason you wanted the same ticks, but different limits that would require something new.

@QuLogic QuLogic modified the milestones: v3.4.0, v3.5.0 Jan 27, 2021
@jklymak
Copy link
Member

jklymak commented Apr 27, 2021

So, I think by default:

import matplotlib.pyplot as plt
import matplotlib as mpl
fig, ax = plt.subplots()
loc = mpl.ticker.AutoLocator()
ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(loc)
ax.set_ylim([0.6, 0.8])
plt.show()

Should warn: locator used on two axis-es, changing state on one axis will change the locators on the other. and preserve the existing behaviour

To suppress this, the user should be able to set a kwarg:

ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(loc, copyloc=False)

suppresses the warning, and behaves like before

ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(loc, copyloc=True)

makes a new locator from the old.

If they don't want to deal with either, they could just do what they probably meant, which was:

ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(copy.copy(loc))

@jklymak
Copy link
Member

jklymak commented Apr 27, 2021

ping @timhoffm , @anntzer re #13482

@timhoffm
Copy link
Member Author

My concerns re #13482 are still there #13482 (comment)

copyloc kwarg: This feels a bit like a crutch. I'd be ok if this is only for a transition period. Is sharing a locator still on the table? If not, well finally arrive at

ax.xaxis.set_major_locator(loc)
ax.yaxis.set_major_locator(copy.copy(loc))

as the only valid variant, which I'd favor.

@anntzer
Copy link
Contributor

anntzer commented May 30, 2021

I really don't think asking users to manually copy NullLocators or FuncFormatters makes sense from a user-friendliness PoV.

@jklymak
Copy link
Member

jklymak commented May 30, 2021

We seem to regularly have this problem where we allow passing complicated objects to methods. Is the same issue with colormaps, scales, etc. I'm not sure what the usual pythonic way of dealing with this is, but it would sure be simpler if these were passed by value rather than address

@anntzer
Copy link
Contributor

anntzer commented May 31, 2021

I think (consistently with above) the correct solution would be to have locators not know about the axis except at call time (locator(axis) instead of locator()), but that runs into the usual backcompat problems (it's all doable, just tedious...)

It may be worth trying to get a (non-exhaustive) list of third-party locators/formatters that we may want to contact/check if we want to do the change.

@QuLogic QuLogic modified the milestones: v3.5.0, v3.6.0 Sep 25, 2021
@QuLogic QuLogic modified the milestones: v3.6.0, v3.7.0 Aug 24, 2022
@timhoffm timhoffm modified the milestones: v3.7.0, future releases Dec 22, 2022
Copy link

github-actions bot commented Apr 9, 2025

This issue has been marked "inactive" because it has been 365 days since the last comment. If this issue is still present in recent Matplotlib releases, or the feature request is still wanted, please leave a comment and this label will be removed. If there are no updates in another 30 days, this issue will be automatically closed, but you are free to re-open or create a new issue if needed. We value issue reports, and this procedure is meant to help us resurface and prioritize issues that have not been addressed yet, not make them disappear. Thanks for your help!

@github-actions github-actions bot added the status: inactive Marked by the “Stale” Github Action label Apr 9, 2025
@timhoffm timhoffm added the keep Items to be ignored by the “Stale” Github Action label Apr 9, 2025
@timhoffm
Copy link
Member Author

timhoffm commented Apr 9, 2025

Making Locators and Formatters stateless seems worth investigating.

It may be worth trying to get a (non-exhaustive) list of third-party locators/formatters that we may want to contact/check if we want to do the change.

A first attempt with still a lot of false-positive is https://github.com/search?q=%2Fc%28%3F-i%29lass+%5Cw%2B%5C%28.*Locator%5C%29%3A%2F+language%3APython+NOT+is%3Afork&type=code

@anntzer
Copy link
Contributor

anntzer commented Apr 9, 2025

@timhoffm Perhaps you can have a look at #28429? It's not the most elegant solution, but it effectively works around the existing design issues while keeping backcompat.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
keep Items to be ignored by the “Stale” Github Action status: inactive Marked by the “Stale” Github Action topic: ticks axis labels
Projects
None yet
Development

No branches or pull requests

6 participants