Skip to content

rust(bug): Fix recursive/nested hdf5 imports#579

Open
Brandon-Shippy wants to merge 17 commits into
mainfrom
fix/recursive-hdf5-import
Open

rust(bug): Fix recursive/nested hdf5 imports#579
Brandon-Shippy wants to merge 17 commits into
mainfrom
fix/recursive-hdf5-import

Conversation

@Brandon-Shippy
Copy link
Copy Markdown
Contributor

@Brandon-Shippy Brandon-Shippy commented May 20, 2026

Description

  • Properly recurses through nested groups to discover all channels in a nested hdf5 file
  • Per group time pairing (1d), each dataset is paired with the timestamp in its group
  • Supports more data types to cleanly import into sift
  • Skips unknown datatypes instead of failing

Verification

  • Added unit tests for walking through groups to find channels properly.
  • All tests pass
  • Manual testing of file types with the nested structure.
  • Imported each type 1d 2d compound and properly structures in sift

Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
Comment thread rust/crates/sift_cli/src/cmd/import/hdf5/detect_hdf5_schema.rs Outdated
@Brandon-Shippy Brandon-Shippy requested a review from solidiquis May 20, 2026 22:07

if group_time.is_empty() {
return Err(anyhow!(
"no time dataset found — expected one of {TIME_NAMES:?} (case-insensitive) \
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

General question, how fixed are the possible time names? Wondering if longer term it could be useful to make these definable through the CLI?

assert_eq!(mapped.len(), 2);
assert_eq!(mapped[0].name, "OFF");
assert_eq!(mapped[0].key, 0);
assert!(mapped[0].is_signed);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should there be a test for an unsigned enum?

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