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

WIP: Enable reading groups from HDF5 #28

Closed
wants to merge 2 commits into from

Conversation

weiji14
Copy link

@weiji14 weiji14 commented Aug 22, 2023

Allow hidefix to read groups in a HDF5 file.

Currently just adding a placeholder unit test that lists the group member names, and added a sample HDF5 file (will delete it or find a smaller one later). I'll refactor this PR once I get my head around Rust a bit more, but feel free to leave comments/suggestions.

Towards fixing #24.

References:

@codecov-commenter
Copy link

Codecov Report

Merging #28 (87f7756) into main (0e63714) will not change coverage.
The diff coverage is n/a.

❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main      #28   +/-   ##
=======================================
  Coverage   57.00%   57.00%           
=======================================
  Files          21       21           
  Lines        1056     1056           
=======================================
  Hits          602      602           
  Misses        454      454           

@gauteh
Copy link
Owner

gauteh commented Aug 22, 2023

Sounds good, I also sometimes download the file during the test: https://github.com/gauteh/hidefix/blob/main/tests/read_norkyst.rs#L12, but those files are much bigger than this one. You could also even generate it using the native hdf5 bindings.

@weiji14
Copy link
Author

weiji14 commented Nov 2, 2023

Sorry, was away with a few conferences/trips the past couple of months, so haven't managed to find time to work on this. I see that there's another PR at #31 now, so happy for that to go ahead and close this one if needed.

@gauteh
Copy link
Owner

gauteh commented Nov 3, 2023

Yeah, I think that one is probably further ahead: but it became outdated after some other changes, so it requires some work before it can be merged (otherwise it is ready).

@gauteh
Copy link
Owner

gauteh commented Nov 18, 2023

Closed in favor of #31.

@gauteh gauteh closed this Nov 18, 2023
@weiji14 weiji14 deleted the read_group branch November 18, 2023 20:09
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.

3 participants