Skip to content

DOC: MaxNLocator and contour/contourf doc update #16428

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
wants to merge 1 commit into from
Closed

DOC: MaxNLocator and contour/contourf doc update #16428

wants to merge 1 commit into from

Conversation

pharshalp
Copy link
Contributor

@pharshalp pharshalp commented Feb 6, 2020

PR Summary

closes #12729

MaxNLocator's docs were incorrect (please see the details described in #12729).

Here is a numerical experiment that I ran to determine the difference between the number of tick locations returned by MaxNLocator and the argument nbins.

import matplotlib.ticker as mticker
import numpy as np

steps = np.linspace(1, 10, 37)   # Overkill -- 1.  ,  1.25,  1.5 ,  1.75,  2.... , 10
vmax_arr = np.linspace(0, 1, 1001)[1:]  # Overkill

# number of ticks - number of bins
nticks_minus_nbins = np.zeros((1001, 40, 3), dtype=np.int64)
for vmax_ind, vmax in enumerate(vmax_arr):
    for nbins in range(1, 40+1):
        locator = mticker.MaxNLocator(nbins, steps=steps, min_n_ticks=1)
        nticks_minus_nbins[vmax_ind, nbins-1, 0] = len(locator.tick_values(0, vmax))-nbins
        nticks_minus_nbins[vmax_ind, nbins-1, 1] = len(locator.tick_values(-vmax, 0))-nbins
        nticks_minus_nbins[vmax_ind, nbins-1, 2] = len(locator.tick_values(-vmax, vmax))-nbins
print(np.max(nticks_minus_nbins))  # returns 2

It turns out (as was shown in the original issue) that MaxNLocator returns at most nbins+2 tick locations (nbins+1 intervals).

Since contour/contourf uses MaxNLocator (as pointed out in the original issue) updated their docs as well.

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

@pharshalp pharshalp requested a review from timhoffm February 6, 2020 20:23
@codecov
Copy link

codecov bot commented Feb 6, 2020

Codecov Report

Merging #16428 into master will decrease coverage by 0.39%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #16428      +/-   ##
==========================================
- Coverage   80.86%   80.46%   -0.40%     
==========================================
  Files         307      307              
  Lines       75749    74407    -1342     
  Branches     9692     9688       -4     
==========================================
- Hits        61252    59875    -1377     
- Misses      11959    11987      +28     
- Partials     2538     2545       +7     
Impacted Files Coverage Δ
lib/matplotlib/backends/backend_macosx.py 4.54% <0.00%> (-36.12%) ⬇️
lib/matplotlib/testing/jpl_units/StrConverter.py 15.55% <0.00%> (-5.28%) ⬇️
lib/matplotlib/testing/jpl_units/EpochConverter.py 69.69% <0.00%> (-4.67%) ⬇️
lib/matplotlib/docstring.py 65.00% <0.00%> (-3.19%) ⬇️
lib/matplotlib/backends/qt_compat.py 48.42% <0.00%> (-3.16%) ⬇️
...b/matplotlib/testing/jpl_units/UnitDblConverter.py 65.85% <0.00%> (-2.33%) ⬇️
lib/matplotlib/backend_managers.py 20.95% <0.00%> (-2.30%) ⬇️
lib/matplotlib/backends/backend_qt5.py 55.94% <0.00%> (-1.44%) ⬇️
lib/matplotlib/font_manager.py 74.10% <0.00%> (-1.17%) ⬇️
lib/matplotlib/pyplot.py 71.72% <0.00%> (-1.14%) ⬇️
... and 152 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 68bdd3e...cf18cf1. Read the comment docs.

@tacaswell tacaswell requested a review from efiring February 6, 2020 21:33
@tacaswell tacaswell added this to the v3.3.0 milestone Feb 6, 2020
@tacaswell
Copy link
Member

The issue seems to be our semi-standard confusion about who's job it is to clip out-of-range ticks:

In [13]: import matplotlib.ticker as mticker
     ... locator = mticker.MaxNLocator(4+1, min_n_ticks=1)
     ... vmin, vmax = (-0.11109, 1-0.1111)
     ... t = locator.tick_values(vmin, vmax)
     ... print(t)
     ... print(f'total ticks {len(t)}')
     ... print(f'ticks in window {sum((vmin < t) * (t < vmax))}')
[-0.2  0.   0.2  0.4  0.6  0.8  1. ]
total ticks 7
ticks in window 5

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.

Please make the proposed change.

Other than that I'd merge on the basis that this is an incremental doc improvement. I don't think it's worth digging deeper into tick clipping as part of this PR.

@pharshalp
Copy link
Contributor Author

pharshalp commented Feb 16, 2020

I messed my github matplotlib repo settings (and not github doesn't recognize my old branch... it says pharshalp wants to merge 1 commit into matplotlib:master from **unknown repository**) so closing this PR and recreating it at #16534.
Sorry for the trouble!

@pharshalp pharshalp closed this Feb 16, 2020
efiring added a commit that referenced this pull request Mar 2, 2020
…_doc

DOC: MaxNLocator and contour/contourf doc update (replaces #16428)
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.

Docs for contour levels argument is incorrect
6 participants