Add tick_values method to the date Locators #4330
Conversation
| vmin *= MUSECONDS_PER_DAY | ||
| vmax *= MUSECONDS_PER_DAY | ||
| ticks = self._wrapped_locator.tick_values(vmin, vmax) | ||
| def __call__(self): |
There was a problem hiding this comment.
Can you add a note to https://github.com/matplotlib/matplotlib/tree/master/doc/api/api_changes about this? It is a harmless change (as the inputs were ignored), but it should still be documented.
|
@has2k1 Thank you, this looks great! Just one small nit about documenting API changes. This is probably also worth documenting in docs/user/whats_new (see the readme file in that folder) to make this change discoverable. |
|
Added the documentation, is it o.k. to rebase and squash? |
|
Yes, doing violence to PRs is acceptable (and sometimes encouraged). |
| @@ -0,0 +1,6 @@ | |||
| Removed `args` and `kwargs` from `MicrosecondLocator.__call__` | |||
There was a problem hiding this comment.
This should be two more layers down in api/api_changes
| ticks = self._wrapped_locator.tick_values(vmin, vmax) | ||
| def __call__(self): | ||
| # if no data have been set, this will tank with a ValueError | ||
| try: |
There was a problem hiding this comment.
Can you also document this API change? It looks like it is making it consistent with the base class so I think that this is OK.
*Issue* Date Locators are a subclass of `matplotlib.ticker.Locator` and they should override the `tick_values` method. This method allows Locators to be queried for tick locations using two limits. Without `tick_values` this only axes with data on them can get tick locations by calling on the `Locator` object. Or the user has to go through the hussle of creating dummy axes, add data to them, then use `Locator`. *Problem* The date Locators have implemented the tick generation logic in the `__call__` method. This is in contrast with the `Locators` implemented in `matplotlib.ticker` for which the `__call__` method gets the limits from the axes and then lets `tick_values` to do the the rest. *Solution* - Have the `__call__` method of the date Locators get the limits and let `tick_values` do the calculations. - Make `MicrosecondLocator.__call__` access the axes using the same helper method that the other date Locators use. - Make sure all the Date Locators return empty lists if the axes are empty instead of some raising `ValueError`s while others do not. - For the tests, `matplotlib.tests.test_dates:test_auto_date_locator` is still appropriate. All the Locators are tested in there.
ENH/API : Add tick_values method to the date Locators
|
@has2k1 Thank you. Congratulations on (what I think) is your first mpl contribution! |
|
@tacaswell thanks too, your comments were clear and helpful. Yes this is my first contribution, after more than a year of working on ggplot I expected to bump into something much earlier. |
Issue
Date Locators are a subclass of
matplotlib.ticker.Locatorand they should override thetick_valuesmethod. This method allows Locators to be queried for tick locations using two limits. Withouttick_valuesthis only axes with data on them can get tick locations by calling on theLocatorobject. Or the user has to go through the hussle of creating dummy axes, add data to them, then use
Locator.Problem
The date Locators have implemented the tick generation logic in the
__call__method. This is in contrast with theLocatorsimplemented inmatplotlib.tickerfor which the__call__method gets the limits from the axes and then letstick_valuesto do the the rest.Solution
__call__method of the date Locators get the limits and lettick_valuesdo the calculations.MicrosecondLocator.__call__access the axes using the same helper method that the other date Locators use.matplotlib.tests.test_dates:test_auto_date_locatoris still appropriate. All the Locators are tested in there.