Skip to content

FIX: make MaxNLocator only follow visible ticks for order of magnitude #12086

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Sep 14, 2018
Merged

FIX: make MaxNLocator only follow visible ticks for order of magnitude #12086

merged 2 commits into from
Sep 14, 2018

Conversation

jklymak
Copy link
Member

@jklymak jklymak commented Sep 10, 2018

PR Summary

Closes #12072

Order of magnitude for tick formatting in ScalarFormatter was using invisible ticks. This changes to just use visible.

Probably obviated by #11004 but won't hurt anything.

Before:

figure_2

After

figure_1

Code

import matplotlib.pyplot as plt
import matplotlib.ticker as ticker

sci_nums = [10**9, 8*10**9]
plt.rcParams['ytick.labelsize'] = 13
fig, ax = plt.subplots(1, 2)
ax[0].scatter(range(len(sci_nums)), sci_nums)
ax[1].scatter(range(len(sci_nums)), sci_nums)

ax[1].yaxis.set_major_locator(ticker.MaxNLocator(4))
plt.show()

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

@QuLogic
Copy link
Member

QuLogic commented Sep 11, 2018

Seems to have broken some docs when there are no visible ticks.

@jklymak
Copy link
Member Author

jklymak commented Sep 11, 2018

Ha, it passes all of test_axes and test_ticker...

@jklymak jklymak force-pushed the fix-changing-orderofmagnitude branch from fc00626 to ed1c830 Compare September 11, 2018 00:12
@anntzer
Copy link
Contributor

anntzer commented Sep 11, 2018

Modulo the docs...

@jklymak
Copy link
Member Author

jklymak commented Sep 11, 2018

@anntzer I think the docs are OK (they say it'll just key off the data, not the data plus one tick on either side). It does need tests, which I'll do now. I actually don't understand why test_ticker passes all the tests... It should have failed some of them. Of course the test for this is all parameterized etc, so it'll take me a bit to grok what is going on..

@jklymak
Copy link
Member Author

jklymak commented Sep 11, 2018

Added test that fails master but passes this PR...

@jklymak jklymak force-pushed the fix-changing-orderofmagnitude branch from 3183f78 to 9a23d6d Compare September 11, 2018 17:41
@NelleV
Copy link
Member

NelleV commented Sep 14, 2018

Thanks @jklymak !
I agree that the docs don't need changing on that one, so I'll go ahead and merge.

@NelleV NelleV merged commit afda25a into matplotlib:master Sep 14, 2018
@QuLogic QuLogic added this to the v3.1 milestone Sep 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@anntzer anntzer anntzer approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
v3.1.0
Development

Successfully merging this pull request may close these issues.

MaxNLocator changes the scientific notation exponent with different number of tick labels
4 participants
@jklymak @QuLogic @anntzer @NelleV