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 our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Datetime64 fixes #816

Merged
merged 13 commits into from Aug 18, 2016

Conversation

Projects
None yet
2 participants
@philippjfr
Member

philippjfr commented Aug 17, 2016

Matplotlib does not currently support plotting numpy datetime64 types directly. This PR adds a compatibility layer ensuring these types are converted appropriately for plotting in matplotlib.

@@ -61,6 +63,14 @@ def get_data(self, element, ranges, style):
ys = element.dimension_values(1)
return (xs, ys), style, {}
def init_artists(self, ax, plot_args, plot_kwargs):
xs, ys = plot_args
if xs.dtype.kind == 'M':

This comment has been minimized.

@jlstevens

jlstevens Aug 17, 2016

Member

Might be worth adding a comment mentioning what the 'M' code means. I assume it is the numpy datetime64 type?

@@ -270,3 +272,10 @@ def dim_axis_label(dimensions, separator=', '):
if not isinstance(dimensions, list): dimensions = [dimensions]
return separator.join([safe_unicode(d.pprint_label)
for d in dimensions])
def dt64_to_dt(dt64):

This comment has been minimized.

@jlstevens

jlstevens Aug 17, 2016

Member

Seems like a useful utility!

@jlstevens

This comment has been minimized.

Member

jlstevens commented Aug 17, 2016

Looks good. Ready to merge?

@philippjfr

This comment has been minimized.

Member

philippjfr commented Aug 17, 2016

Think there might be a few more places where datetime64 support can be improved. Will investigate a bit more.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Aug 18, 2016

I've now overhauled the PR again. The main changes here are that you can now register datetime formatters for datetime64 and datetime types, which means you can now do this:

%%opts Curve [xrotation=15 xticks=5]
import numpy as np
from datetime import date
import pandas as pd

start = date(2012, 1, 15)
end = date(2012, 1, 30)
hv.Dimension.type_formatters[np.datetime64] = '%m/%d'
hv.Curve((pd.date_range(start, end, freq='D'), np.random.rand(16)))

image

Additionally I fixed various bugs in xarray date range handling and made formatting of grid tick labels consistent with everything else. Ready to merge now, but there's a definite possibility tests will break due to Grid tick formatting changes.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Aug 18, 2016

Ready to review, updated test data will pass shortly.

@@ -262,9 +262,6 @@ class GridPlot(CompositePlot):
show_legend = param.Boolean(default=False, doc="""
Legends add to much clutter in a grid and are disabled by default.""")
tick_format = param.String(default="%.2f", doc="""

This comment has been minimized.

@jlstevens

jlstevens Aug 18, 2016

Member

So this parameter is now deprecated?

This comment has been minimized.

@philippjfr

philippjfr Aug 18, 2016

Member

Hmm, yes, we have a system for tick formatting using Dimension.value_format and Dimension.type_formatters, so this is redundant. You think we need to go through a deprecation cycle for this?

This comment has been minimized.

@jlstevens

jlstevens Aug 18, 2016

Member

It will at least need to be mentioned in the CHANGELOG. Other than that I think it is fine.

return value.strftime(formatter)
elif isinstance(value, np.datetime64):
return dt64_to_dt(value).strftime(formatter)
elif re.findall(r"\{(\w+)\}", formatter):
return formatter.format(value)

This comment has been minimized.

@jlstevens

jlstevens Aug 18, 2016

Member

So the datetime64 objects will use datetime style formatting? That is fine, assuming datetime64 doesn't have its own formatting conventions.

This comment has been minimized.

@philippjfr

philippjfr Aug 18, 2016

Member

Yes, I think that's nice behavior, datetime64 can't be formatted directly so it converts to regular datetime first and then applies the formatter.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Aug 18, 2016

Looks good. I've made two comments and once you have addressed those, I am happy to merge.

@jlstevens jlstevens merged commit 2b5e4a0 into master Aug 18, 2016

3 of 4 checks passed

continuous-integration/travis-ci/push The Travis CI build failed
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage decreased (-0.04%) to 46.683%
Details
s3-reference-data-cache Test data is cached.
Details

@jlstevens jlstevens removed the in progress label Aug 18, 2016

@philippjfr philippjfr deleted the datetime64_fixes branch Sep 2, 2016

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