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

Use HFile instead of h5py.File in most places #4792

Merged
merged 29 commits into from
Jul 5, 2024

Conversation

GarethCabournDavies
Copy link
Contributor

@GarethCabournDavies GarethCabournDavies commented Jun 24, 2024

Use pycbc.io.hdf's HFile class rather than h5py.File directly so that we can make changes to the functionality.

For example checksumming, version information, and anything else we may want

Standard information about the request

This is a change to utilise potential future features
This change touches everywhere in the code, but affects nothing

This change follows style guidelines (See e.g. PEP8), has been proposed using the contribution guidelines

Contents

Not quite find-and-replace, but update everywhere which uses h5py.File to pycbc.io.HFile

Links to any issues or associated PRs

#1525 #4768

Testing

Broadly, this should be checked by the CI running --help for all executables

  • The author of this pull request confirms they will adhere to the code of conduct

@GarethCabournDavies
Copy link
Contributor Author

Note that I haven't touched bin/plotting/pycbc_plot_bank_bins, as that is already subclassing h5py.File, though I'm not sure whether the functionality which is there already exists elsewhere

@GarethCabournDavies GarethCabournDavies changed the title DRAFT: Use hfile everywhere instead of h5py.File Use HFile instead of h5py.File in most places Jun 27, 2024
@GarethCabournDavies
Copy link
Contributor Author

I have also avoided live (due to efficiency worries) and inference, so as not to interfere with the h5py.File wrapper used there

Copy link
Contributor

@spxiwh spxiwh left a comment

Choose a reason for hiding this comment

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

I'm okay with this, any issues @ahnitz ?

Copy link
Member

@ahnitz ahnitz left a comment

Choose a reason for hiding this comment

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

Yeah, I don't have a problem with this.

@GarethCabournDavies GarethCabournDavies merged commit 3483ad5 into gwastro:master Jul 5, 2024
33 checks passed
@GarethCabournDavies GarethCabournDavies deleted the use_hfile branch July 5, 2024 09:02
yi-fan-wang pushed a commit to yi-fan-wang/pycbc that referenced this pull request Jul 15, 2024
* Move to HFile in pycbc_add_statmap

* Move to HFile in pycbc_apply_rerank

* update all-sky psd code

* Move pycbc_coinc_(findtrigs,hdfinjfine,mergetrigs) to HFile

* More places with HFile

* More moves

* Use HFile in more places

* Use HFile in more places

* Use HFile in more places

* Use HFile in more places

* Use HFile in more places

* Use HFile rather than h5py.File in bank

* move live executables to use HFile

* Use HFile in live examples

* Move minifollowups to use HFile

* Move populations to use HFile

* Move banksim codes to use HFile

* Start to move plotting codes to use HFile

* Finish moving plotting codes to use HFile

* Move scripts in /bin/ to use HFile

* Move bin/workflow_comparisons/offline_search codes to use HFile

* Use HFile in pycbc/events

* Moving more parts of pycbc/ to use HFile

* Use HFile for other things in pycbc/io/hdf

* Use HFile in pygrb codes

* Revert "Use HFile in pycbc/events"

This reverts commit 99dce9d.

* Missed removal of imports

* Remove HFile from types/array

* Remove HFile from pycbc/types
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