Skip to content

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

Merged
NelleV merged 2 commits into
matplotlib:masterfrom
jklymak:fix-changing-orderofmagnitude
Sep 14, 2018
Merged

FIX: make MaxNLocator only follow visible ticks for order of magnitude#12086
NelleV merged 2 commits into
matplotlib:masterfrom
jklymak:fix-changing-orderofmagnitude

Conversation

@jklymak

@jklymak jklymak commented Sep 10, 2018

Copy link
Copy Markdown
Member

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

QuLogic commented Sep 11, 2018

Copy link
Copy Markdown
Member

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

@jklymak

jklymak commented Sep 11, 2018

Copy link
Copy Markdown
Member Author

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

anntzer commented Sep 11, 2018

Copy link
Copy Markdown
Contributor

Modulo the docs...

@jklymak

jklymak commented Sep 11, 2018

Copy link
Copy Markdown
Member Author

@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

jklymak commented Sep 11, 2018

Copy link
Copy Markdown
Member Author

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

NelleV commented Sep 14, 2018

Copy link
Copy Markdown
Member

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