FIX: always use at least 2 ticks and recompute #5772

tacaswell · 2015-12-31T16:57:38Z

For extreme aspect-ratio plots the auto ntick logic would decide that
no ticks will fit, leading to divide by 0 issue.

This changes the logic in Axis.get_tick_space to have a floor of 1.
also does not cache the number of ticks so that if the on-screen
aspect ratio changes the number of ticks will update correctly.

I tried to add a test for this, but something about the testing framework is causing trouble

import matplotlib.pyplot as plt

fig, ax = plt.subplots()
ax.set_aspect('equal')
ax.set_xlim(0, 1000)
fig.canvas.draw()
print(ax.yaxis.get_major_locator()())
print(ax.xaxis.get_major_locator()())
assert len(ax.yaxis.get_major_locator()()) == 2
ax.set_ylim(0, 1000)
fig.canvas.draw()
assert len(ax.yaxis.get_major_locator()()) > 2

runs correctly as a script for me and interactively, but as part of the test suite give 6 ticks in the first case.

mdboom · 2015-12-31T17:58:32Z

Tests in the test suite have a different figure size. You can set the size manually when creating the figure, however.

tacaswell · 2015-12-31T19:15:25Z

duh, rcparams for the tests use 'classic' so this functionality is turned off!

WeatherGod · 2015-12-31T19:57:55Z

Test failure:

ImageComparisonFailure: images not close: /home/travis/build/matplotlib/matplotlib/result_images/test_axes_grid1/inset_locator.png vs. /home/travis/build/matplotlib/matplotlib/result_images/test_axes_grid1/inset_locator-expected.png (RMS 2.177)

tacaswell · 2015-12-31T20:45:49Z

I can reproduce the inset locator failure locally, looking into it now.

tacaswell · 2015-12-31T21:20:03Z

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).

The simplest solution is to change the test code to not use the auto tick count in the test.

Also, kicking my self for merging the PR where this test got added for not asking Mike to change to 'viridis' for the color map.

tacaswell · 2016-01-02T02:08:39Z

I think this is ready for review.

mdboom · 2016-01-04T15:01:02Z

lib/matplotlib/axis.py

@@ -2004,7 +2004,7 @@ def get_tick_space(self):
            # is no more than 3:1
            size = tick.label1.get_size() * 3
            size *= np.cos(np.deg2rad(tick.label1.get_rotation()))
-            self._tick_space = np.floor(length / size)
+            return np.floor(length / size)


So we're no longer caching the calculation? This could be somewhat expensive to do every time. If not too expensive, we should remove the if self._tick_space (and the self._tick_space member) altogether. If it is, maybe we just need to properly invalidate this value using the stale infrastructure (which will be traitlets sometime in the near future)?

tacaswell · 2016-01-16T22:09:04Z

@jenshnielsen Finally made the last change to this, it should be ready to go

jenshnielsen · 2016-01-17T13:47:07Z

Looks good to me. I think the changes to test_axes_grid1 should be implemented in the example that this is based on too i.e. inset_locator_demo2.py and possibly others

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.

tacaswell · 2016-01-24T20:11:54Z

@jenshnielsen examples updated.

jenshnielsen · 2016-01-24T20:16:54Z

cool I will review and merge asap

FIX: always use at least 2 ticks and recompute

tacaswell added this to the next major release (2.0) milestone Dec 31, 2015

tacaswell added the status: needs review label Dec 31, 2015

tacaswell closed this Dec 31, 2015

tacaswell reopened this Dec 31, 2015

tacaswell added status: needs review and removed status: needs review labels Dec 31, 2015

tacaswell mentioned this pull request Dec 31, 2015

Set maximum number of bins in ticking #5724

Merged

tacaswell force-pushed the min_nticks branch from d1e294c to ea62a77 Compare December 31, 2015 20:45

This was referenced Jan 3, 2016

dynamic ticking (#5588) should avoid (if possible) single ticks #5784

Closed

Refresh pgf baseline images. #5727

Merged

mdboom reviewed Jan 4, 2016
View reviewed changes

tacaswell mentioned this pull request Jan 10, 2016

Recompute figsize-dependent parameters on resize #5824

Closed

tacaswell force-pushed the min_nticks branch from 03339b9 to 6d9efe3 Compare January 24, 2016 19:45

tacaswell added 4 commits January 24, 2016 14:47

TST: test that number of ticks adjust with aspect

f257244

MNT: remove caching of _tick_space

57623d6

tacaswell force-pushed the min_nticks branch from 6d9efe3 to 57623d6 Compare January 24, 2016 19:49

DOC: modify examples to fix number of ticks

bc359be

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.

jenshnielsen added a commit that referenced this pull request Jan 25, 2016

Merge pull request #5772 from tacaswell/min_nticks

7cea22c

FIX: always use at least 2 ticks and recompute

jenshnielsen merged commit 7cea22c into matplotlib:v2.x Jan 25, 2016

mdboom removed the status: needs review label Jan 25, 2016

tacaswell deleted the min_nticks branch April 11, 2016 19:55

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FIX: always use at least 2 ticks and recompute #5772

FIX: always use at least 2 ticks and recompute #5772

tacaswell commented Dec 31, 2015

mdboom commented Dec 31, 2015

tacaswell commented Dec 31, 2015

WeatherGod commented Dec 31, 2015

tacaswell commented Dec 31, 2015

tacaswell commented Dec 31, 2015

tacaswell commented Jan 2, 2016

mdboom Jan 4, 2016

tacaswell commented Jan 16, 2016

jenshnielsen commented Jan 17, 2016

tacaswell commented Jan 24, 2016

jenshnielsen commented Jan 24, 2016

FIX: always use at least 2 ticks and recompute #5772

FIX: always use at least 2 ticks and recompute #5772

Conversation

tacaswell commented Dec 31, 2015

mdboom commented Dec 31, 2015

tacaswell commented Dec 31, 2015

WeatherGod commented Dec 31, 2015

tacaswell commented Dec 31, 2015

tacaswell commented Dec 31, 2015

tacaswell commented Jan 2, 2016

mdboom Jan 4, 2016

Choose a reason for hiding this comment

tacaswell commented Jan 16, 2016

jenshnielsen commented Jan 17, 2016

tacaswell commented Jan 24, 2016

jenshnielsen commented Jan 24, 2016