8000 Merge v2x to master by jenshnielsen · Pull Request #5906 · matplotlib/matplotlib · GitHub
[go: up one dir, main page]

Skip to content

Merge v2x to master #5906

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

Merged
merged 8 commits into from
Jan 25, 2016
Merged

Conversation

jenshnielsen
Copy link
Member

Replaces #5905

tacaswell and others added 7 commits January 24, 2016 14:47
For extreme aspect-ratio plots the auto ntick logic would decide that
no ticks will fit, leading to divide by 0 issue.

 - In ticker ensure there is always at least on bin
 - Axis do not cache the number of ticks so that if the on-screen
   aspect ratio changes the number of ticks will update correctly.
Do not let the ticks in the inset adjust based on spacing of text which
is not visible.

The issue with the inset locator seems to be that as previously
implemented the axis's estimate of how many ticks it should have was
computed once and then saved.

The way that the inset axes works (if you step through this line at a
time) is that the figure-space size of the axes follows the x/y limits
at a fixed ratio to the screen-space of the same range in the main
axes. Thus, when the inset axes is created it is 'big' in that it has
limits of [0, 1] at 6x zoom -> about half the size of the host axes and
so it concludes that it needs a whole bunch of ticks and remembers that.

One of the changes in this PR makes it so that the number of ticks is
recomputed every time, thus many fewer ticks are used when the axes is
small (even though the text is all turned off).
In the inset axes (which will not have tick labels) fix the number
of ticks instead of letting auto-ticker pick the number of ticks.
FIX: always use at least 2 ticks and recompute
@@ -4282,9 +4282,16 @@ def _helper_y(ax):


@cleanup
def test_broken_barh_empty():
def test_adjust_numtick_aspect():
Copy link
Member Author

Choose a reason for hiding this comment

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

It looks like this removes the test_broken_barh_empty test because the rest accidentally got duplicated in an earlier merge

@jenshnielsen
Copy link
Member Author

There seems to be a problem in the tight layout demo

/home/travis/build/matplotlib/matplotlib/doc/examples/pylab_examples/demo_tight_layout.rst:8: WARNING: Exception occurred in plotting demo_tight_layout
 from /home/travis/build/matplotlib/matplotlib/doc/mpl_examples/pylab_examples/demo_tight_layout.py:
Traceback (most recent call last):
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/sphinxext/plot_directive.py", line 654, in render_figures
    figman.canvas.figure.savefig(img.filename(format), dpi=dpi)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py", line 1698, in savefig
    self.canvas.print_figure(*args, **kwargs)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backend_bases.py", line 2232, in print_figure
    **kwargs)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 520, in print_png
    FigureCanvasAgg.draw(self)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/backends/backend_agg.py", line 467, in draw
    self.figure.draw(self.renderer)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 62, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/figure.py", line 1292, in draw
    func(*args)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 62, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axes/_base.py", line 2392, in draw
    a.draw(renderer)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/artist.py", line 62, in draw_wrapper
    draw(artist, renderer, *args, **kwargs)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py", line 1126, in draw
    ticks_to_draw = self._update_ticks(renderer)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py", line 959, in _update_ticks
    tick_tups = [t for t in self.iter_ticks()]
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py", line 959, in <listcomp>
    tick_tups = [t for t in self.iter_ticks()]
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/axis.py", line 904, in iter_ticks
    self.major.formatter.set_locs(majorLocs)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/ticker.py", line 551, in set_locs
    self._set_offset(d)
  File "/home/travis/build/matplotlib/matplotlib/lib/matplotlib/ticker.py", line 567, in _set_offset
    ave_oom = math.floor(math.log10(np.mean(np.absolute(locs))))
ValueError: cannot convert float NaN to integer

@mdboom
Copy link
Member
mdboom commented Jan 25, 2016

This fixes it for me:

diff --git a/lib/matplotlib/ticker.py b/lib/matplotlib/ticker.py
index 2d416bb..7455d03 100644
--- a/lib/matplotlib/ticker.py
+++ b/lib/matplotlib/ticker.py
@@ -563,7 +563,7 @@ class ScalarFormatter(Formatter):
         locs = np.asarray(locs)
         locs = locs[(vmin <= locs) & (locs <= vmax)]
         ave_loc = np.mean(locs)
-        if ave_loc:  # dont want to take log10(0)
+        if len(locs) and ave_loc:  # dont want to take log10(0)
             ave_oom = math.floor(math.log10(np.mean(np.absolute(locs))))
             range_oom = math.floor(math.log10(range))

But I have no idea why it's failing when merged onto master and not on v2.x alone.

@mdboom
Copy link
Member
mdboom commented Jan 25, 2016

Maybe that docs are built on Python 2.7 on v2.x and 3.5 on master?

@jenshnielsen
Copy link
Member Author

That looks sensible. For some reason I cannot reproduce the issue outside Sphinx. If I build the docs locally whit python 3.5 I see the failure but not if I run the example separately

@mdboom
Copy link
Member
mdboom commented Jan 25, 2016

Interesting. I was able to reproduce the problem separately, for what it's worth.

@jenshnielsen
Copy link
Member Author

I guess it could depend on default figure size maybe? Anyway I pushed your suggested fix to this branch so lets see what travis thinks.

@jenshnielsen
Copy link
Member Author

There is still something wrong. This is my local build from Sphinx of one of the figures. Note that there is only one tick. This is the merge of @tacaswell's pr which should ensure at least 2 ticks always.

demo_tight_layout_00_04

When build locally it looks like this
figure_5

@@ -1434,7 +1434,7 @@ def set_params(self, **kwargs):
def bin_boundaries(self, vmin, vmax):
nbins = self._nbins
if nbins == 'auto':
nbins = min(self.axis.get_tick_space(), 9)
nbins = max(min(self.axis.get_tick_space(), 9), 1)
Copy link
Member

Choose a reason for hiding this comment

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

Does this need to be "2" at the end? (Just a wild guess...)

Copy link
Member

Choose a reason for hiding this comment

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

Sounds like a reasonable guess, but I think that at least originally, nbins was a maximum number of intervals, so the maximum number of ticks would be one larger; but all of this is dealing with maxima, not with any guaranteed number.

Copy link
Member

Choose a reason for hiding this comment

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

I stated with 2 and change to 1 for exactly the reason @efiring stated.

@jenshnielsen
Copy link
Member Author

The failure is indeed a python 2 vs 3 issue.

@jenshnielsen
Copy link
Member Author

This is ultimately caused by differences in math.floor in python 2 and 3

Python 3

a = np.NaN
math.floor(a)
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-4-9e269286b82d> in <module>()
----> 1 math.floor(a)

ValueError: cannot convert float NaN to integer

Python 2

a = np.NaN
math.floor(a)
nan

@jenshnielsen
Copy link
Member Author

So this should be ready to merge but f05d1d0 should be backported to 2.x but there still appears to be a bug in that we only get one tick in the docs build of the figure because the other one is clipped out.

tacaswell added a commit that referenced this pull request Jan 25, 2016
@tacaswell tacaswell merged commit 7d1a7c2 into matplotlib:master Jan 25, 2016
@QuLogic QuLogic added this to the 2.1 (next point release) milestone Jan 25, 2016
tacaswell added a commit that referenced this pull request Jan 26, 2016
@jenshnielsen jenshnielsen deleted the mergev2xmaster branch February 16, 2016 09:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants
0