Skip to content
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

H5 Minute Reader Bug in Python 3 #2680

Merged
merged 1 commit into from
Apr 2, 2020
Merged

H5 Minute Reader Bug in Python 3 #2680

merged 1 commit into from
Apr 2, 2020

Conversation

dmichalowicz
Copy link
Contributor

If an equities or futures update file was written in python 2 but read in python 3, pandas.read_hdf would error trying to read the tz attr in the file.

@@ -35,7 +35,7 @@ flask-cors==2.1.3
flask==1.1.1 # via flask-cors
funcsigs==1.0.2 # via mock, python-interface
futures==3.2.0 # via tornado
h5py==2.7.1
h5py==2.10.0
Copy link
Contributor Author

@dmichalowicz dmichalowicz Apr 1, 2020

Choose a reason for hiding this comment

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

This update was necessary to read the file's attr in the tests...2.7.1 was failing with a strange numpy c error.

@coveralls
Copy link

coveralls commented Apr 1, 2020

Coverage Status

Coverage decreased (-0.3%) to 87.885% when pulling a99401e on read-hdf-fix-py3 into e4eff16 on master.

@dmichalowicz dmichalowicz force-pushed the read-hdf-fix-py3 branch 4 times, most recently from 5a8ae0f to a0916f8 Compare April 1, 2020 21:09

# Our current version of h5py is unable to read the tz attr in the
# tests as it was written by HDFStore. This is fixed in version
# 2.10.0 of h5py, but that requires >=Python3.7, so until then we
Copy link
Contributor

Choose a reason for hiding this comment

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

I think 2.10 only requires python 3.7 on conda.

Copy link
Contributor

@ssanderson ssanderson left a comment

Choose a reason for hiding this comment

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

@dmichalowicz the logic here looks good to me. The only comment I had was that your comment currently says that we need python 3.7 to use the newer version of h5py. I don't think that's actually true in general, that's just for conda-based installs.

@dmichalowicz
Copy link
Contributor Author

Right, thanks, I'll fix and then merge.

@dmichalowicz dmichalowicz merged commit 129a448 into master Apr 2, 2020
@dmichalowicz dmichalowicz deleted the read-hdf-fix-py3 branch April 2, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants