Skip to content

[Bug]: fig.subplots_adjust and ax.set_yticklabels together can produce unexpected results #26398

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

Closed
foldvaridominic opened this issue Jul 27, 2023 · 10 comments · Fixed by #26401
Labels
Community support Users in need of help.
Milestone

Comments

@foldvaridominic
Copy link

foldvaridominic commented Jul 27, 2023

Bug summary

Setting y ticks and labels directly on an axis and then adjusting the top of the subplot of the figure can have unexpected consequences. Depending on where those ticks and labels were originally, and how the top ratio is set, labels will not go until the top of the y-axis, instead all the values appear with much less vertical spacing on the bottom part of the axis.

Code for reproduction

import matplotlib.pyplot as plt
from mpl_toolkits.axisartist import SubplotHost

fig = plt.figure(figsize=(25, 3))  # Originally setting a large enough height fixed our problem, but in this example it did not help

ax = SubplotHost(fig, 111)
fig.add_subplot(ax)
# ax = fig.add_subplot(111)  # using this produces the same outcome

data = [33.4, 33.5, 33.6]

ax.plot(data, data)

ticks = [32.8, 33., 33.2, 33.4, 33.6, 33.8]

# Anomaly because labels won't match with ticks anymore if the next line is commented out
# ax.set_yticks(ticks)
ax.set_yticklabels(map(str, ticks))

# In this little example the bug is only triggered when the anomaly above holds
fig.subplots_adjust(top=0.96)

plt.show(dpi=300)

Actual outcome

image

Expected outcome

image

Additional information

I suspect it depends on where those ticks and labels were originally, because we were also able to produce this bug where there was no mismatch required between labels and ticks.
It might also depend on the number of the top ratio being set.
As far as I know, adjusting other ratios (left, right, bottom, wspace, hspace) did not have an effect on this.

Operating system

Ubuntu 22.04

Matplotlib Version

3.5.1

Matplotlib Backend

agg

Python version

3.9.12

Jupyter version

No response

Installation

pip

@foldvaridominic foldvaridominic changed the title [Bug]: fig.subplots_adjust and ax.set_yticklabels together can produce unexpected results [Bug]: fig.subplots_adjust and ax.set_yticklabels together can produce unexpected results if height is too small Jul 27, 2023
@foldvaridominic foldvaridominic changed the title [Bug]: fig.subplots_adjust and ax.set_yticklabels together can produce unexpected results if height is too small [Bug]: fig.subplots_adjust and ax.set_yticklabels together can produce unexpected results Jul 27, 2023
@jklymak
Copy link
Member

jklymak commented Jul 27, 2023

Setting manual tick labels does not set manual ticks. It just labels the first n ticks where n is your number of tick labels. We suggest using set_ticks with the label argument to prevent confusion

@jklymak jklymak added the Community support Users in need of help. label Jul 27, 2023
@foldvaridominic
Copy link
Author

foldvaridominic commented Jul 27, 2023

Setting manual tick labels does not set manual ticks. It just labels the first n ticks where n is your number of tick labels. We suggest using set_ticks with the label argument to prevent confusion

Yes, that part is clear. In reality set_ticklabels is called only to format the original labels at their original location. I just needed a minimal example that reproduces the original bug. The set ticks part is omitted deliberately in this example code. Otherwise the bug disappears.

@rcomer
Copy link
Member

rcomer commented Jul 27, 2023

If you just want to format the labels for automatically placed ticks, you might want to set a tick formatter, rather than explicitly setting labels:
https://matplotlib.org/stable/gallery/ticks/tick-formatters.html

@jklymak
Copy link
Member

jklymak commented Jul 27, 2023

import numpy as np
from matplotlib import cm
import matplotlib.pyplot as plt

for adjust in [False, True]:
    fig, ax = plt.subplots(figsize=(5, 3))
    data = [33.4, 33.5, 33.6]
    ax.plot(data, data)
    ticks = [32.8, 33., 33.2, 33.4, 33.6, 33.8]
    # ax.set_yticks(ticks)
    # ax.set_yticklabels(map(str, ticks))

    if adjust:
        fig.subplots_adjust(top=0.96)
        fig.savefig('Adjust.png')
    else:
        fig.savefig('DontAdjust.png')

If you adjust, you get twice as many ticks:

Don't Adjust

DontAdjust

Adjust

Adjust

You then label them with the un-adjuested (too few) labels:

AdjustLabel

You further get a Warning

UserWarning: FixedFormatter should only be used together with FixedLocator
  ax.set_yticklabels(map(str, ticks))

I think the warning could be less mysterious. Note that https://matplotlib.org/stable/api/_as_gen/matplotlib.axes.Axes.set_xticklabels.html is discouraged in the docs.

@tacaswell
Copy link
Member

To summarize:

  • set_xticklabels switches to using FixedFormammter which maps your N strings to the first n ticks. In some limited cases this is useful (e.g. a stand alone script where you can trust the number of ticks won't change because the next thing you do is save without a layout manager)
  • adjusting the axes size in the OP's example happens to fall across an edge where the auto locator decides to increase the number of ticks. Changing the figure size or running one of the layout engines has a good chance of generating the same issue (or reducing the number of ticks and making essentially the same error but in a less detectable way).

So I think our options are:

  1. do nothing, we are warning and this is, although a foot-gun, expected behavior
  2. make the warning clearer / special case it in set_ticklabels to give more context as to how to fix it
  3. just...fix it? We can detect when we are setting a FixedFormatter with not-a-FixedLocator...can we just grab the current tick locations, set a FixedLocator, and warn loudly?
  4. deprecate set_ticklabels because it is a persistent foot-cannon
  5. make it raise instead of warn

Of the options, I'm leaning towards 3. If we do just "fix it", I suspect that it can be done in a way that has 0 effect on people who were getting lucky and protect users from falling into this trap. 1 is not great (it is biting users), 2 is like the status quo but just more helpfully broken, 4 and 5 will break a bunch of users for whom things are currently "working" (and these may be low-Matplotlib-expertise users as they are using something we advise against and are currently ignoring a warning).

@jklymak
Copy link
Member

jklymak commented Jul 27, 2023

I don't understand how you will "just fix it"? That's what we do now I think: There are six ticks and the user specifies four labels so two end up blank.

In special cases we could infer the tick values from the strings, but that's not true in general.

I started a PR to just have a specific warning in set_ticklabels that doesn't refer to mysterious classes like FixedFormatter.

@tacaswell
Copy link
Member

In the case where there are N ticks and the user gives us N labels, we can change what ever locator is there into a FixedLocator with where the ticks currently are

@jklymak
Copy link
Member

jklymak commented Jul 27, 2023

I believe that is what we already do. The issue here is that the number of ticks changes after the user has set the tick labels when the Axes is made larger.

@tacaswell
Copy link
Member

We set a FixedFormammter, if we had also set a FixedLocator then the tick locations should not have changed

@jklymak
Copy link
Member

jklymak commented Jul 28, 2023

You mean if the set_ticks happens before the resize? That could work if the order of calls is correct. I think we'd have to trigger a draw to get the ticks from the AutoLocator at that point?

@QuLogic QuLogic added this to the v3.8.0 milestone Aug 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Community support Users in need of help.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants