Skip to content

Issue252 - dealing with user blocks in front of HDF5 superblocks - #258

Open
bnlawrence wants to merge 8 commits into
mainfrom
issue252
Open

Issue252 - dealing with user blocks in front of HDF5 superblocks#258
bnlawrence wants to merge 8 commits into
mainfrom
issue252

Conversation

@bnlawrence

Copy link
Copy Markdown
Collaborator

Description

#252 exposed a problem with our superblock handling. The HDF5 spec allows a user block in front of the HDF5 itself, and HDF5 implementations should look for the superblock at 0, and then at powers of 2 greater than 512 bytes. This means that if we don't find a superblock at offset zero, we have to go looking for where it might be before rejecting the file as not being an HDF5 file.

Closes #252

Checklist

  • This pull request has a descriptive title and labels
  • This pull request has a minimal description (most was discussed in the issue, but a two-liner description is still desirable)
  • Unit tests have been added (if codecov test fails)
  • Any changed dependencies have been added or removed correctly (if need be)
  • If you are working on the documentation, please ensure the current build passes
  • All tests pass

@codecov

codecov Bot commented Aug 27, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 82.60870% with 16 lines in your changes missing coverage. Please review.
✅ Project coverage is 78.59%. Comparing base (667c468) to head (f5fa7d7).

Files with missing lines Patch % Lines
pyfive/h5d.py 68.75% 5 Missing ⚠️
pyfive/utilities.py 84.00% 3 Missing and 1 partial ⚠️
pyfive/dataobjects.py 76.92% 3 Missing ⚠️
pyfive/high_level.py 93.33% 1 Missing and 1 partial ⚠️
pyfive/misc_low_level.py 71.42% 1 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #258      +/-   ##
==========================================
+ Coverage   78.48%   78.59%   +0.11%     
==========================================
  Files          15       15              
  Lines        3416     3485      +69     
  Branches      546      555       +9     
==========================================
+ Hits         2681     2739      +58     
- Misses        593      602       +9     
- Partials      142      144       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@bnlawrence

Copy link
Copy Markdown
Collaborator Author

@davidhassell When you do the review, the only consideration I have is whether or not we should do an if in the high-level file initialisation and only add the HDF5OfffsetWrapper if the base-address is not zero. That way we will avoid a lot of subtracting of zeros for normal files ...

@bnlawrence

Copy link
Copy Markdown
Collaborator Author

Actually, I think I should just do this. Fix incoming.

@bnlawrence
bnlawrence marked this pull request as ready for review August 28, 2026 11:34

@davidhassell davidhassell left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi Bryan,

Nice job. All makes sense, and I can see that it optimised in the "not constantly adding zero when there's no user block" sense.

The test is simple but effective.

decode_vlen

This also looks OK, in terms of the tests look sensible and pass, and the core logic makes sense.

Should we also not the in the PR that this fixes #228?

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.

Support for Matlab HDF5 files?

2 participants