-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
Update _base.py #16141
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
Update _base.py #16141
Conversation
Adds an additional note to clarify get_xticklabels
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.
Blocking on comment below, but feel free to clear if I'm misunderstanding something. I think this should be turned into an issue that should be fixed rather than something that should be worked around.
lib/matplotlib/axes/_base.py
Outdated
Notes | ||
----- | ||
Please note that the tick label list is not populated until ``plt.draw()`` has been called. | ||
See also: https://stackoverflow.com/questions/32700935/get-xticklabels-contains-empty-text-instances |
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.
I dont' think we usually call out to outside resources (they are fragile). If there is something pertinent in that post please include here.
I think this is actually a bug, and the lazy tick instantiation should be triggered by get_xticklabels
. I think before we did this lazily, this used to work? ping @anntzer @efiring
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.
I raised an issue for this (#16140). I'd be happy to expand on that and provide a minimal example if it helps.
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.
I think we never did (at least AFAIK) and the current behavior was always present, but I agree we could consider changing it.
I also agree the SO link could be skipped.
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.
I cannot remember of this ever being different than it is now. It's pretty inherent to the fact that the locator first needs to determine the ticks and the formatter finding their labels. This can only be done when drawing the axes - else, it might still change afterwards.
For that reason I would actually argue for not changing the behaviour; because as it stands, we can at least be sure that get_xticklabels()
will never return anything that is only seemingly correct.
Adding a note in the docs seems useful though. (No SO link needed IMO. Instead a reference to pyplot.draw
and canvas.draw
)
Also, I would dare to claim that in 90% of the cases where people might want to use .get_{x,y}ticklabels
, there would be a better solution. (E.g. in the linked SO post, they want to rotate labels, for which one does not actually need to know the ticklabels.)
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.
Well, then I think this leads to the question: what is the point of this function at all? Presumably its to get the tick labels and change them (i.e. add a dollar sign or a happy face to all of them). If the user wants to do this, and avoid writing their own formatter, then I'd argue they shouldn't need to call an explicit draw
in order to do so. Or, if we do need to call a draw (because it's impossible to know the limits before a draw) then maybe this function should call the draw
. But I think the function is fundamentally broken if we have to explicitly tell users to call draw
to get it to work.
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.
At least before 3.0 ax.get_position()
used to give wrong positions unless you drew the canvas. Not sure what exactly changed in between. But to construct an example where this is still the case, we can use an inset axes:
import matplotlib.pyplot as plt
import numpy as np
fig, ax = plt.subplots()
ax.imshow(np.random.rand(10,4))
axins = ax.inset_axes([0.0, 0.4, 0.7, 0.5])
print(axins.get_xticklabels())
# prints "wrong" labels: <a list of 6 Text xticklabel objects>
print(axins.get_position())
# prints "wrong" position: Bbox(x0=0.125, y0=0.418, x1=0.6675, y1=0.803)
fig.canvas.draw()
print(axins.get_xticklabels())
# prints "correct" labels: <a list of 3 Text xticklabel objects>
print(axins.get_position())
# prints "correct" position: Bbox(x0=0.397, y0=0.418, x1=0.5587, y1=0.803)
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.
OK, but thats a pretty specialized example using set_axes_locator
. I agree that any axes that use `set_axes_locator (including everything in axes_grid) will not return a sensible position until the locator function is called.
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.
... though come to think of it, its possible the axes_locator function could be called if get_position()
is called.
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.
Hi everyone,
I've updated the documentation improvement suggestion:
- removed the link to SO
- added references to
pyplot.draw
andcanvas.draw
I guess the same problem will also apply to the yticklabels
method? A colleague (@christian-stepanek) and I had the idea that if the function should not be changed, maybe it would at least be helpful to add a warning message so the user is informed that the tick label list has not yet been populated. Would something like this be feasible?
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.
Yes, set_yticklabels
should get the same text. 👍
I'm -0.5 on adding a warning. We tend to be cautious to issue warnings when we are not sure something is going wrong. It's annoying to issue warnings for valid use cases. There are (few) cases when one might want to have the tick Text
instances and the actual string values are irrelevant.
lib/matplotlib/axes/_base.py
Outdated
|
||
Notes | ||
----- | ||
Please note that the tick label list is not populated until a ``draw`` |
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.
Maybe more precisely:.. the tick label strings are not not...
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.
OK
Please note that the tick label strings are not populated until a ``draw`` | ||
method has been called. | ||
|
||
See also: `~pyplot.draw` and `~canvas.draw`. | ||
""" |
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.
You'll need to fix these references for CI to be happy...
Co-Authored-By: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
Co-Authored-By: Tim Hoffmann <2836374+timhoffm@users.noreply.github.com>
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.
Thanks @pgierz ! This looks great 👍
Adds an additional note to clarify get_xticklabels
PR Summary
PR Checklist