-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
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
Merge v2x to master #5906
Conversation
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(): |
There was a problem hiding this comment.
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
There seems to be a problem in the tight layout demo
|
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. |
Maybe that docs are built on Python 2.7 on v2.x and 3.5 on master? |
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 |
Interesting. I was able to reproduce the problem separately, for what it's worth. |
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. |
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. |
@@ -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) |
There was a problem hiding this comment.
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...)
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
The failure is indeed a python 2 vs 3 issue. |
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 |
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. |
Replaces #5905