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

Made datetime handling in plotting code more general #1098

Merged
merged 3 commits into from Feb 2, 2017

Conversation

Projects
None yet
3 participants
@philippjfr
Member

philippjfr commented Feb 2, 2017

Improves datetime handling for datetime.datetime and pd.tslib.Timestamp types. Needs a bunch of unit tests.

@philippjfr philippjfr added the bokeh label Feb 2, 2017

@philippjfr

This comment has been minimized.

Member

philippjfr commented Feb 2, 2017

Very happy with this refactor the code is no longer duplicated and now handles datetime types more consistently. Also added a bunch of unit tests just now. Ready to merge when tests pass.

@philippjfr philippjfr requested a review from jlstevens Feb 2, 2017

@jlstevens

This comment has been minimized.

Member

jlstevens commented Feb 2, 2017

Ok, the tests have now passed. The changes look good and I am very happy to see the code simplified and the addition of new tests.

Merging.

@jlstevens jlstevens merged commit 68c6af8 into master Feb 2, 2017

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.06%) to 77.991%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the datetime_handling branch Feb 10, 2017

@stonebig

This comment has been minimized.

Contributor

stonebig commented Apr 22, 2017

hi small remark. testing with holoviews latest build, I get this error:

  • holoviews-1.7.dev8-py3-none-any.whl
  • pandas-0.20.0rc1-cp35-cp35m-win_amd64.whl
# Holoviews 
# for more example, see http://holoviews.org/Tutorials/index.html
import holoviews as hv
%load_ext holoviews.ipython
fractal = hv.Image(image)

((fractal * hv.HLine(y=0.16)).hist() + fractal.sample(y=0.16))
C:\WinPython\basedir35\buildQt5\winpython-64bit-3.5.x.2\python-3.5.3.amd64\lib\site-packages\holoviews\core\util.py:26: FutureWarning: pandas.tslib is deprecated and will be removed in a future version.
You can access Timestamp as pandas.Timestamp
  datetime_types = datetime_types + (pd.tslib.Timestamp,)

..; and the image is not displayed (on Jupyterlab-0.20.0), but that may not be an holoviews problem

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 22, 2017

Thanks for the note, I'll make a PR with the fix now.

@stonebig

This comment has been minimized.

Contributor

stonebig commented Apr 22, 2017

further down, i see errors in statsmodels "The pandas.core.datetools module is deprecated and will be removed in a future version. Please use the pandas.tseries module instead.
from pandas.core import datetools"... maybe a full check would be nice

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 22, 2017

further down, i see errors in statsmodels "The pandas.core.datetools module is deprecated and will be removed in a future version. Please use the pandas.tseries module instead.
from pandas.core import datetools"... maybe a full check would be nice

Could this be triggered by some other dependency? I think it's probably seaborn, because holoviews does not import statsmodels directly.

@stonebig

This comment has been minimized.

Contributor

stonebig commented Apr 22, 2017

it's not triggered by holloviews, just meant that there may be a lot more deprecation warnings to chaze per holoviews with pandas-0.20.0rc1

@philippjfr

This comment has been minimized.

Member

philippjfr commented Apr 22, 2017

I see, yes that makes sense, I'll try it out.

@stonebig

This comment has been minimized.

Contributor

stonebig commented Apr 22, 2017

oups ! it's a new kind of deprecation for me "FutureWarning". I new "DeprecationWarning" and "UserWarning" up to now.

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