BUG: invert_xaxis (negative tot_sec) triggers MilliSecondLocator (#3990)

[jorisvandenbossche]

Solves issue #3990.
Negative timedelta for xaxis range will no longer trigger MilliSecondLocator.

[jorisvandenbossche]

Do you think this is a correct fix?
And do I have to include some more things (test, mention in release notes, ..)

[jreback]

didn't see this....does this resolve the issue?

@cpcloud can you take a look?

[cpcloud]

there must be a reason that they are negative

@jorisvandenbossche can you explain why this fixes the problem? the relativedelta is allowed to be negative, but on the other hand, maybe whoever wrote this didn't take that into account.

[cpcloud]

e.g.

In [19]: a,b=date_range('20010101', periods=2)

In [20]: from dateutil.relativedelta import relativedelta

In [21]: d=relativedelta(a,b)

In [22]: delta = d

In [23]: paste
        num_days = ((delta.years * 12.0) + delta.months * 31.0) + delta.days
        num_sec = (delta.hours * 60.0 + delta.minutes) * 60.0 + delta.seconds
        tot_sec = num_days * 86400. + num_sec
## -- End pasted text --

In [24]: tot_sec
Out[24]: -86400.0

that doesn't like a case of "i forgot to abs", i could be wrong though.

[jorisvandenbossche]

For me this resolves the issue (but I submitted the patch of course :-) Although I don't really know if I could write a test for it.

relativedelta is indeed allowed to be negative, and is negative in the case of an inversed xaxis. That is not the problem. However, when deciding which tick Locator we want to use, it doesn't matter if it is inversed or not: we are interested in the absolute time delta. Because now the negative delta triggers a MilliSecondLocator just because it is negative, while the time delta is not suited for Milliseconds.

Some more details with references to code:
When using ax.invert_xaxis() the left and right xlim are interchanged. Because of this the delta in PandasAutoDateLocator will be negative (https://github.com/pydata/pandas/blob/master/pandas/tseries/converter.py#L241).
This in turn triggers MilliSecondLocator instead of the normal AutoDateLocator (because tot_sec in line https://github.com/pydata/pandas/blob/master/pandas/tseries/converter.py#L247 is negative), and causes the error as the datetime range is too big for MilliSecondLocator.

[cpcloud]

makes sense

@jreback i think this is ok.

[cpcloud]

@jorisvandenbossche thanks for the explanation!

[jreback]

ok..
@jorisvandenbossche can you add a quick release note?

[jorisvandenbossche]

OK, will do when I get home.

Thanks for the look at it!

[jorisvandenbossche]

I updated the commit with release notes. So for me it is ready to merge.

As I said in the issue, the only thing I don't understand is why the bug does not appear when using pandas plot method to create the axes object (so ax.plot(df.index, df['col'].values) vs df.plot(ax=ax))

[jreback]

ok....merging...thansk...keep us apprised if there are any addtl issues, or if you figure out the above (you can't open in issue if you want)