Rework MaxNLocator, eliminating infinite loop; closes #6849 #6919

Merged
merged 3 commits into from Aug 22, 2016

Conversation

Projects
None yet
3 participants
Owner

efiring commented Aug 7, 2016

The core algorithm has been reimplemented in a more compact and understandable form than my earlier try at adding the min_n_ticks constraint.

I replaced the images for one mplot3d test because I think the present behavior is more consistent than the earlier behavior that led to those images. In the replaced versions, all of the axes have ticks just slightly inside the limits. Based on the gallery it appears this is the intended behavior but @WeatherGod can confirm or deny this.

@efiring efiring Rework MaxNLocator, eliminating infinite loop; closes #6849
0648b73

efiring added the needs_review label Aug 7, 2016

Owner

efiring commented Aug 8, 2016

Here also, the appveyor py 3.5 build failed early on; it's unrelated to this PR.

@tacaswell tacaswell commented on the diff Aug 9, 2016

lib/matplotlib/ticker.py
@@ -1662,6 +1662,13 @@ def set_params(self, **kwargs):
steps = list(steps)
steps.append(10)
self._steps = steps
+ # Make an extended staircase within which the needed
+ # step will be found. This is probably much larger
+ # than necessary.
+ flights = (0.1 * np.array(self._steps[:-1]),
@tacaswell

tacaswell Aug 9, 2016

Owner

👍 to the naming

Contributor

anntzer commented Aug 9, 2016

This leads to integer=True not being respected in the following case:

ax = gca(); ax.locator_params(integer=True); ax.set_xlim(-.1, 1.1)

(or the example in #6849) even though the ticker could very well just use the two ticks at the two integer values.

@efiring efiring fix handling of smallest integer interval
688fbee
Owner

efiring commented Aug 9, 2016

@anntzer you are correct with respect to your example; it is now fixed and works as I intended. The example in #6849, however, leaves room for only one integer tick, so it will relax the integer constraint (given the default min_n_ticks=2).

@efiring efiring Add test of MaxNLocator integer option.
f4e4c10

@tacaswell tacaswell merged commit b2a25b2 into matplotlib:master Aug 22, 2016

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.08%) to 70.779%
Details

tacaswell removed the needs_review label Aug 22, 2016

@tacaswell tacaswell added a commit that referenced this pull request Aug 22, 2016

@tacaswell tacaswell Merge pull request #6919 from efiring/integer_locator
ENH: Rework MaxNLocator, eliminating infinite loop; closes #6849
9982e40
Owner

tacaswell commented Aug 22, 2016

backported to v2.x as 9982e40

QuLogic referenced this pull request Dec 7, 2016

Closed

Validate steps input to `MaxNLocator` #7578

4 of 5 tasks complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment