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

extract time step from hdf5 directly #145

Merged
merged 1 commit into from
Feb 21, 2017

Conversation

PrometheusPi
Copy link
Member

@PrometheusPi PrometheusPi commented Feb 21, 2017

This pull request solves issue #144 by extracting the time step / iteration from hdf5 directly and not from the file name.

However, extracting the time step from hdf5 files is slower.

iteration = int(list(f['/data'].items())[0][0])
f.close()
# Create list of tuples (which can be sorted together)
iters_and_names.append((iteration, full_name))
Copy link
Member

Choose a reason for hiding this comment

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

uh, append all found iterations, not only the first one :-)

@ax3l
Copy link
Member

ax3l commented Feb 21, 2017

cool, let's see if we can even open a groupBased file now: openPMD/openPMD-converter#4

@PrometheusPi PrometheusPi force-pushed the fix_iteration_list_h5_files branch 2 times, most recently from 6ce61a3 to 21f28b4 Compare February 21, 2017 16:31
@RemiLehe RemiLehe self-assigned this Feb 21, 2017
@RemiLehe
Copy link
Member

@PrometheusPi : Thanks, that was fast!
The automated tests indicate that there is a minor linting issue. Could you try running

pep8 --ignore=E201,E202,E122,E127,E128,E131 opmd_viewer

and fix the issues?

Otherwise, the PR is fine with me; thanks again!

@PrometheusPi
Copy link
Member Author

@RemiLehe Thanks for pointing out the pep8 issues - I think I fixed them.
Now it fails here with a return value of 256.

@RemiLehe
Copy link
Member

The automated tests return the following error:

File "/home/travis/miniconda2/envs/testing/lib/python3.6/site-packages/openPMD_viewer-0.5.2-py3.6-linux-x86_64.egg/opmd_viewer/openpmd_timeseries/utilities.py", line 50, in list_h5_files

    iters_and_names.append((int(str_iteration), full_name))

TypeError: int() argument must be a string, a bytes-like object or a number, not 'Group'

My guess is that .items() should be replaced by .keys(). Could you check?

Note: you can run the automated tests on your local computer by doing:

python setup.py install
python setup.py test

@RemiLehe
Copy link
Member

Also, I'm not sure if [0] is needed. On the whole, I guess the line

iterations = list(f['/data'].items())[0]

should be replaced by

iterations = list(f['/data'].keys())

But again, I have not checked.

@PrometheusPi
Copy link
Member Author

@RemiLehe Thanks!
Using .keys() is even simpler and faster - thanks for the input.

@RemiLehe
Copy link
Member

Great, I'll merge. Thanks for the PR.

Of course, if you need this feature in production, you'll need to install the dev version, at least until our next release.

@RemiLehe RemiLehe merged commit 03978e2 into openPMD:dev Feb 21, 2017
@PrometheusPi
Copy link
Member Author

@ax3l I tested the groupBased file and it works like a charm 👍

@PrometheusPi PrometheusPi deleted the fix_iteration_list_h5_files branch February 22, 2017 12:57
@ax3l
Copy link
Member

ax3l commented Feb 22, 2017

awesome, thanks! 👯‍♂️

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

Successfully merging this pull request may close these issues.

3 participants