Skip to content

Fixes pyplot xticks() and yticks() by allowing setting only the labels #15788

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

Conversation

rscmendes
Copy link

@rscmendes rscmendes commented Nov 28, 2019

PR Summary

This PR was suggested in issue #15005 as to fix the error that is returned when xticks() or yticks() is called with only the labels as parameters.

The changes remove the error by allowing setting only the labels. This is useful for cases where we plot multiple boxplots or bar plots in the same figure, in where we know exactly how many boxplots or bars to plot. The xticks are always range(n), where n is the number of boxplots or bars to put in the plot.

Simple test example:

import matplotlib.pyplot as plt
import numpy as np

data = [np.random.rand(5), np.random.rand(5)]
plt.boxplot(data)
plt.xticks(labels=['first', 'second'])   # fails on current pyplot with TypeError: object of type 'NoneType' has no len(). Fixed in this PR
plt.show()

plt.figure()
plt.boxplot(data, vert=False)
plt.yticks(labels=['first', 'second'])  # fails on current pyplot with TypeError: object of type 'NoneType' has no len(). Fixed in this PR
plt.show()

Note: I didn't add any example to the examples in the function description. Do you think it would be valuable?

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

Copy link
Member

@timhoffm timhoffm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that four your example, one should rather use plt.boxplot(labels, data).

I think there are still cases in which pure label setting is reasonable. Though, I'm slightly concerned that people might abuse this freedom and end up with labels at unexpected positions.

Usually this would need an update of the docstring, but there's #15789 which does already cover that.

@timhoffm timhoffm added this to the v3.3.0 milestone Nov 28, 2019
@timhoffm timhoffm mentioned this pull request Nov 28, 2019
6 tasks
@anntzer
Copy link
Contributor

anntzer commented Nov 28, 2019

This is technically a new feature and needs to be mentioned in the whatsnew.

Given that setting labels without positions is already possible with ax.set_xticklabels, I wouldn't worry about exposing it in pyplot as well.

@rscmendes rscmendes force-pushed the fix-ticks-labels-for-issue-15005 branch from 3035a0a to 31f332e Compare November 29, 2019 15:31
@rscmendes
Copy link
Author

Note that four your example, one should rather use plt.boxplot(labels, data).

I think there are still cases in which pure label setting is reasonable. Though, I'm slightly concerned that people might abuse this freedom and end up with labels at unexpected positions.

Indeed, my example was not good. I was checking my code that originated this suggestion and noticed that I set xticks through plt because I was plotting inside a for cycle. However, I could've just set the labels inside the for cycle using the arg positions. As for stacked bar graphs, one can use the tick_label arg.

So, it might be the case that there's no real use case for this change, except for possibly convenience.

I did some tests with bar graphs, and this addition can actually lead to unexpected results. Differently from boxplots, bar graphs don't always present one xtick per bar. Thus, setting the labels without locs can lead to unexpected (from the user point of view) positioned labels. Here's an example from https://matplotlib.org/3.1.1/gallery/lines_bars_and_markers/bar_stacked.html:

By changing plt.xticks(ind, ('G1', 'G2', 'G3', 'G4', 'G5')) to plt.xticks(('G1', 'G2', 'G3', 'G4', 'G5')), one reaches:
image
There's an xtick at -1 that is created, but not visible in the figure, which causes 'G1' to not show. This is even worse when the number of bars is less than 5, because multiple xticks between bars exist.

To conclude, this change might not be the best solution to this problem. The alternative to this is to return a different error message when setting labels without locs. What do you think it's the best course of action?

@jklymak
Copy link
Member

jklymak commented Nov 29, 2019

We test if ticks are in the view limit; see #12158 So I guess this function could work to check the current view limit and set the ticks based on what is in view. Maybe the test in #12158 should be factored into a private method so we can use it elsewhere rather than having multiple implementations. But I think that API would be clear enough....

@rscmendes
Copy link
Author

rscmendes commented Nov 29, 2019

That would fix the previous example. However, I don't think it would fix the underlying problem, which is clear in this example:

image

bar plots don't generate a single xtick per bar by default, and thus, setting the xticks without the ticks parameter can have this effect.

PS: It seems that the docs generation failed for the added file. I'll fix this asap. A commit squash might be need, apologies.

@anntzer
Copy link
Contributor

anntzer commented Nov 29, 2019

There are certainly problems with setting tick labels, but again xticks is basically repro'ing the API of set_xticklabels (and set_xticks) so I don't think this should prevent us from exposing the API at the pyplot level.

@jklymak
Copy link
Member

jklymak commented Nov 29, 2019

So I guess I’m starting to lean against this PR. It seems rife for confusion to allow the user to specify the labels but not the ticks they expect them attached to. So labels would be optional, but values would not. But I’ve not gone back and parsed all the discussion.

@timhoffm
Copy link
Member

timhoffm commented Dec 1, 2019

One can indeed argue that setting labels without ticks is only really needed in rare circumstances and often creates confusion. It would thus also be a valid choice to not support it in the high-level pyplot API, but require to use Axes.ticklabels in such a case.

@anntzer
Copy link
Contributor

anntzer commented Dec 1, 2019

Technically even Axes.set_xticklabels wasn't needed, we could just have said "Manually create and set a FixedFormatter", but that was not the design choice taken...
I don't feel very strongly about this, though.

@rscmendes
Copy link
Author

So instead of this PR, we should probably give a better error message for when the user tries to set the labels without positions (as suggested by timhoffm). Currently it outputs TypeError: object of type 'NoneType' has no len(), which is not very informative.

Would this be a preferred solution?

@timhoffm
Copy link
Member

timhoffm commented Dec 3, 2019

Put on the agenda of the dev call today.

@efiring
Copy link
Member

efiring commented Dec 3, 2019

I am strongly opposed to allowing labels only, and hence in favor of improving the error message. "Labels-only" is a gun pointed straight at the users foot.

@timhoffm
Copy link
Member

timhoffm commented Dec 3, 2019

The consensus is that that we do not want to expose stetting labels only on the pyplot interface. It's hardly needed and a common source of mistakes. Instead, we should error out cleanly.

@bluetrickpt Would you like to contribute a PR with a better error message?

@rscmendes
Copy link
Author

Sure, I can do that either today or tomorrow. I'll probably do on a different PR though, to clean the commit's log.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants