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

Introduce HDF file cache #83

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
Open

Introduce HDF file cache #83

wants to merge 13 commits into from

Conversation

pmrv
Copy link
Contributor

@pmrv pmrv commented Oct 9, 2024

With a new context manager CachedHDF, users can request a file handle from h5io_browser that stays open for the duration of manager. The context manager can be nested. To implement this I added a new function open_hdf5 and replaced all calls to _open_hdf5. By default this function does nothing but check whether a requested file is already in the cache. If instructed so cache=True it uses the context manager to create a new cache entry and returns that.

I augmented to test suite to run the tests where it makes sense under the caching context as well.

pmrv and others added 13 commits August 30, 2024 09:38
Do the check and retrieve in the same lock context.  Monkey patching as well.
Wrap handles in a nullcontext so that calling code can use

>>> with open_hdf(...) as handle:
...   ...

as before without accidentally closing the file handle
If passed a str, the previous code opened the file, but didn't close or
return it, therefor leaking it.
Instead I'll add a code path in open_hdf that checks if the file is
closed and reopens under a warning as necessary.
Treat compatible modes correctly and do not mix a/r+.
Using this method the clean up is called after tearDown and that causes
an error on windows.
@coveralls
Copy link

coveralls commented Oct 9, 2024

Pull Request Test Coverage Report for Build 11256387758

Details

  • 71 of 75 (94.67%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.3%) to 95.04%

Changes Missing Coverage Covered Lines Changed/Added Lines %
h5io_browser/base.py 52 56 92.86%
Files with Coverage Reduction New Missed Lines %
h5io_browser/base.py 3 90.22%
Totals Coverage Status
Change from base Build 11228669905: -0.3%
Covered Lines: 709
Relevant Lines: 746

💛 - Coveralls

Comment on lines +324 to +326
store = h5py.File(name=filename, mode=mode, libver="latest", swmr=False)
if not store.swmr_mode:
store.swmr_mode = True
Copy link
Member

Choose a reason for hiding this comment

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

Is this change necessary? The previous code was based on the example from the h5py documentation:
https://docs.h5py.org/en/stable/swmr.html#using-the-swmr-feature-from-h5py

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