Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Locator interface #1064

Merged
merged 6 commits into from Aug 16, 2012

Conversation

Projects
None yet
3 participants
Member

pelson commented Aug 9, 2012

I have a need to get ticks for a given range, and the matplotlib Locator seemed like the perfect fit. Unfortunately, the Locator class needs an Axis to function (even if that Axis is a DummyAxis instance).

This PR makes it easy for some Locators to expose their underlying cleverness without the need for an Axis instance.

Owner

mdboom commented Aug 9, 2012

Looks good. Maybe add a unit test and example of how to use this in the new way (without an Axes)? Not sure where that example should go -- it's non-visual (well, non-plot-producing) so it might be hard to find.

@efiring efiring commented on the diff Aug 9, 2012

lib/matplotlib/ticker.py
@@ -913,7 +935,17 @@ def __init__(self, locs, nbins=None):
self.nbins = max(self.nbins, 2)
def __call__(self):
- 'Return the locations of the ticks'
+ return self.tick_values(None, None)
+
+ def tick_values(self, vmin, vmax):
@efiring

efiring Aug 9, 2012

Owner

vmin -> dmin etc. for consistency

@pelson

pelson Aug 10, 2012

Member

For self consistency I would change all signatures in this module from dmin to vmin. Is there a consistency outside of this file which would make dmin preferable?

Owner

efiring commented Aug 9, 2012

I like this idea--I never liked the dummy axis.
Is there any reason not to remove most or all of the separate call definitions? It looks like they could be inherited from the base class.

@pelson pelson commented on an outdated diff Aug 10, 2012

lib/matplotlib/ticker.py
def __call__(self):
- 'Return the locations of the ticks'
+ """Return the locations of the ticks"""
+ # note: some locators return data limits, other return view limits,
+ # hence there is no *one* interface to call self.tick_values.
@pelson

pelson Aug 10, 2012

Member

@efiring said:

Is there any reason not to remove most or all of the separate call definitions? It looks like they could be inherited from the base class.

Unfortunately some __call__ methods use self.axis.get_view_interval others use self.axis.get_data_interval() so I couldn't comfortably make that change.

Member

pelson commented Aug 10, 2012

Looks good. Maybe add a unit test and example of how to use this in the new way (without an Axes)?

Yes, I'd be happy to put in a unit test, but not sure a usage example would be that helpful/interesting.

Member

pelson commented Aug 13, 2012

Ok. I've just added some tests for some of the Locators, but have not updated the "whats new" section as I am not sure such a menial thing is worthy of such exposure. Happy to make any further alterations though if either of you feel it is necessary.

Member

pelson commented Aug 13, 2012

Will just rebase before it gets reviewed.

Owner

mdboom commented Aug 14, 2012

The example I was thinking of would just be something to show how, given a range, one could get a list of ticks back. You've made it much easier than before, so we should show how easy it is... ;)

Member

pelson commented Aug 15, 2012

Ok. changelog added.

@efiring efiring merged commit 4526865 into matplotlib:master Aug 16, 2012

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment