NotificationsYou must be signed in to change notification settings
Fork8.1k
Star22k

WIP: ENH: autodecode pandas timestamps#10638

New issue

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

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

tacaswell merged 1 commit intomatplotlib:masterfromjklymak:enh-autodecode-pandastimestamp

Mar 3, 2018

Merged

WIP: ENH: autodecode pandas timestamps#10638

tacaswell merged 1 commit intomatplotlib:masterfromjklymak:enh-autodecode-pandastimestamp

Mar 3, 2018

Conversation

Copy link

Member

PR Summary

attempt at#10533 after@efiring 's suggestion.

The following now works. (needs a recent pandas for the deregister step to work).

ping@jorisvandenbossche - if you have a more complete or canonical PR, feel free to push instead of this....

importpandasaspdimportmatplotlib.pyplotaspltimportnumpyasnppd.plotting.deregister_matplotlib_converters()xticks=pd.date_range(start="10/10/2017",end="14/10/2017",freq='D')print(type(xticks))print(xticks.to_pydatetime())plt.plot(xticks,np.arange(len(xticks)))plt.xticks(xticks,xticks.strftime("%a %m-%d"))plt.show()

PR Checklist

Has Pytest style unit tests
Code is PEP 8 compliant
New features are documented, with examples if plot related
Documentation is sphinx and numpydoc compliant
Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

jklymak added this to thev2.2.0 milestone

Mar 1, 2018

jklymak added third-party integration topic: date handling topic: units and array ducktypes labels

Mar 1, 2018

anntzer reviewed

Mar 1, 2018

View reviewed changes

lib/matplotlib/dates.py Outdated

		For details see the module docstring.
		"""

		ifhasattr(d,"to_pydatetime"):

Copy link

Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

add a comment that this is to give pandas a special treatment here? (and below)

efiring reviewed

Mar 1, 2018

View reviewed changes

lib/matplotlib/dates.py Outdated

		"""

		ifhasattr(d,"to_pydatetime"):
		d=d.to_pydatetime()

Copy link

Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I think we are unnecessarily losing efficiency here; to avoid that, use the same 2-part conditional here as you use in get_converter. That way, if the input is basically datetime64 it can be handled by your nice, efficient, fully vectorized converter instead of making a round-trip through the clunky datetime.datetime machinery.

Copy link

MemberAuthor

jklymakMar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Can pandas go to datetime64? Ity my understanding that the underlying data type is datetime64[ns], but I don't know what attribute to test for to get that to come out...

Copy link

MemberAuthor

jklymakMar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Oh, duh, I see...

jklymak force-pushed theenh-autodecode-pandastimestamp branch fromcaa6de5 to333b2eaCompare

March 1, 2018 18:37

jorisvandenbossche reviewed

Mar 1, 2018

View reviewed changes

Copy link

jorisvandenbossche left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Thanks for this!
I don't have a PR or branch yet (was planning to do that this evening). I was thinking to go the way to check for thedtype, but anyway, I think there are multiple options that are not necessarily better or worse.

lib/matplotlib/dates.py Outdated

		ifhasattr(d,"values"):
		d=d.values
		ifhasattr(d,"to_pydatetime"):
		d=d.to_pydatetime()

Copy link

jorisvandenbosscheMar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

I am not sure this one is needed, asdate2num already handles Timestamp objects (them being datetime.datetime subclasses):

In [82]: date2num(pd.Timestamp('2012-01-01'))Out[82]: 734503.0In [85]: date2num(datetime.datetime(2012, 1, 1))Out[85]: 734503.0

jklymak force-pushed theenh-autodecode-pandastimestamp branch 2 times, most recently from752e403 to16ac8b7Compare

March 1, 2018 20:26

efiring reviewed

Mar 1, 2018

View reviewed changes

lib/matplotlib/units.py Outdated

		# DISABLED if cached is not None: return cached

		ifhasattr(x,"values"):
		# this unpacks pandas datetime objects...

Copy link

Member

efiringMar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

It is actually much more general than that; it is returning the underlying ndarray from a pandas Series or DataFrame.

Copy link

MemberAuthor

jklymakMar 1, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Great!

jklymak force-pushed theenh-autodecode-pandastimestamp branch from16ac8b7 tocecf1f5Compare

March 1, 2018 22:29

efiring approved these changes

Mar 1, 2018

View reviewed changes

Copy link

Member

efiring left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others.Learn more.

Let's give it a try.

Copy link

MemberAuthor

jklymak commentedMar 1, 2018

I do wonder if this should be in 2.2, but I guess pandas is eager to drop matplotlib as an unconditional dependency?

Copy link

Member

efiring commentedMar 1, 2018

It changes behavior for base mpl only in the case where an object with a ".values" attribute is supplied. The only danger I see there is that someone might have an ndarray subclass with a ".values" attribute that does something other than return the ndarray. So, it's fairly safe for mpl. It could be made even safer by checking to ensure .values really has returned an ndarray, but I doubt this will be necessary, at least for now.

I can imagine that it might have an undesired effect when pandas is in use, by always calling .values instead of simply passing the pandas object on, possibly to be handled by a registered converter that would be looking for the original object. It would be good to have a pandas person look at it from that standpoint. I don't know whether pandas does, or would, register such a converter.

jklymak force-pushed theenh-autodecode-pandastimestamp branch fromcecf1f5 to873addfCompare

March 1, 2018 23:14

Copy link

MemberAuthor

jklymak commentedMar 1, 2018

Oooops, yes, the old version will bypass the pandas converter, beacuse I put the check in the wrong spot (not where you told me to). I think that was a mistake born of another mistake.

This newly pushed version still works w/ the above example, and has the converter after pandas should have chosen a converter...

ENH: autodecode pandas timestamps

59bbccb

jklymak force-pushed theenh-autodecode-pandastimestamp branch from873addf to59bbccbCompare

March 1, 2018 23:17

Copy link

jorisvandenbossche commentedMar 1, 2018

I will take a closer look tomorrow from the pandas side. But, if pandas registers converters, it registers the same converters for both pandas.Timestamp as datetime.datetime and np.datetime64. So I don't think we have to worry about bypassing the pandas converter,if it is registered.

It might be good to add a few tests for this (I suppose youcan directly use pandas there to actually test it?)