Repository navigation
ENH: add rcParam for ConciseDate and interval_multiples - #17022
Conversation
| units.registry[datetime.date] = DateConverter() | ||
| units.registry[datetime.datetime] = DateConverter() | ||
|
|
||
| def register_converters(): |
There was a problem hiding this comment.
This logic should really be in a _validate_date_converter validator, no? otherwise that means that setting the rcParam from anywhere but the matplotlibrc would have no effect.
(And yes, that means we may either need to try to import matplotlib.dates in rcsetup, or, if that's not possible, special case initial import time and re-validate once we import matplotlib.dates).
There was a problem hiding this comment.
Does a validator get called every time the rcParam dict is updated? I guess that would be a good way to do that, if a little magical.
There was a problem hiding this comment.
Is there an easy way to tell if the validation is the initial one or a subsequent one? Because if not, I'm not convinced this is worth the bother.
|
I agree this seems reasonable, but share @anntzer 's concern that as written it won't trigger when the rcparams are changed. |
|
Unless someone has a straightforward way to update the converter during the init of the rcParams without importing datetime, I'm not convinced the magic of being able to change the rcParam on the fly is worthwhile. People can just call the new |
|
ok, but in that case you need at least to block updating of the rcParam once matplotlib is imported (again, with a custom validator) (and even then I doubt I'd accept the feature). I don't like that solution but I think it's expected (well, at least I expect) that setting a variable in the matplotlibrc is equivalent to setting it in python code, so if it's not at least it should error out cleanly. |
|
Well again is there an easy way to know if the validator is being run for first time? If not I don’t see how this can work. Frankly I think the magic imbued in rcParams is needlessly complicated and we should not think of it as something that must allow real time toggles for everything. |
|
It's not so much called once as called from the matplotlibrc, I'm sure the right amount of stack walking could work. |
|
OK, not ridiculously ugly. I just bail if the matplotlib.dates import fails... |
|
Much better :) |
1a6da7b to
0b05b77
Compare
d22ebe4 to
f2a84b1
Compare
f2a84b1 to
e4c7774
Compare
|
... I forgot about this one. Its now rebased and should be ready for review... |
6cb6516 to
22923c5
Compare
|
|
||
| class _rcParam_helper: | ||
| """ | ||
| Never instatiated, but manages the rcParams for dates... |
There was a problem hiding this comment.
perhaps merge the comment above and the docstring...
| converter = DateConverter | ||
|
|
||
| interval_multiples = cls.int_mult | ||
| print(interval_multiples) |
|
|
||
| interval_multiples = cls.int_mult | ||
| print(interval_multiples) | ||
| units.registry[np.datetime64] = converter( |
There was a problem hiding this comment.
perhaps just instantiate a single converter and assign to all 3?
There was a problem hiding this comment.
... I guess so? the old code instantiated three converters, so I'm a little hesitant. OTOH, I'm not sure why it did, and I don't think we lose any flexibility this way...
| try: | ||
| import matplotlib.dates as mdates | ||
| mdates._rcParam_helper.set_converter(s) | ||
| except Exception as e: |
There was a problem hiding this comment.
add a comment that this can only fail at initial import time?
There was a problem hiding this comment.
Yeah - so this gives me a bit of pause. Does this cause any import slowdowns?
There was a problem hiding this comment.
I guess you could write mdates = sys.modules.get("matplotlib.dates"); if mdates: ... as matplotlib.dates will be in sys.modules once everything is set up properly...
There was a problem hiding this comment.
Thanks, I prefer that....
There was a problem hiding this comment.
@jklymak do you still want to change how this is written?
anntzer
left a comment
There was a problem hiding this comment.
Needs perhaps a few comments/docstring fixes, but other than that looks fine to me.
6b6f120 to
2cdcfe7
Compare
jklymak
left a comment
There was a problem hiding this comment.
Add doc in ConciseDateFormatter
tacaswell
left a comment
There was a problem hiding this comment.
Modulo a example with a plot in the whats new.
|
|
||
| def __init__(self, formats=None, zero_formats=None, offset_formats=None, | ||
| show_offset=True): | ||
| show_offset=True, interval_multiples=True): |
|
|
||
| @staticmethod | ||
| def axisinfo(unit, axis): | ||
| def __init__(self, interval_multiples=True): |
| plt.rcParams['date.converter'] = 'auto' | ||
| plt.rcParams['date.interval_multiples'] = False | ||
| ax = axs[1] | ||
| ax.plot(dates, y) |
There was a problem hiding this comment.
This renders as https://39494-1385122-gh.circle-artifacts.com/0/doc/build/html/users/next_whats_new/rcparams_dates.html I'm forgetting how to get the plot to show in the docs? OK figured it out
dfd8924 to
4f29425
Compare
4f29425 to
abc06c5
Compare
Ooops, duh - I missed one of the two cases. Thanks for pointing that out. |
abc06c5 to
610f5fb
Compare
610f5fb to
a793bde
Compare
PR Summary
This is a proposal for two new
datercParam entries:date.converterwhich takes a string. If its'concise'thenmdates.ConciseDateConverteris used for dates, otherwisemdates.AutoDateConverteris used (as before). Default is the same as before: 'auto'date.interval_multiplessets whether 'interval_multiples' is used (True by default) for the auto locators.Note this needs a newEDIT: fixed to allow automatic changing of the rcParams....mdates.register_convertersto work aftermatplotlib.dateshas been imported bymatplotlib.pyplot.Needs tests and a what's new...
PR Checklist