fix(format): keep the array dimensions of compound v1 members
Compound datatype version 1 carries, per member, a dimensionality and four dimension sizes (HDF5 before 1.4 had no array class). The parser skipped those 28 bytes, so a member such as `f: f32[4]` came back as a single f32 at the member's offset: the compound's size was right but its members were wrong. libhdf5 wraps such a member in an array type of the first `dimensionality` sizes and ignores the permutation; do the same, and reject a dimensionality above 4 as libhdf5 does. Only files old enough to also use layout message v1 have these, so this became reachable with the previous commit (tarrold.h5, tcompound.h5). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
@@ -300,6 +300,11 @@
|
||||
- Two threads reading two chunked datasets through one `File` could get each
|
||||
other's chunks (the shared chunk cache was switched between datasets
|
||||
across separate lock acquisitions). The cache is now keyed by dataset.
|
||||
- Compound datatype version 1 members with legacy array dimensions (HDF5
|
||||
before 1.4, which had no array class) were read as a single scalar at
|
||||
the member's offset; they are now array members, as in libhdf5
|
||||
(`tarrold.h5`, `tcompound.h5`). Only reachable once layout versions 1/2
|
||||
were readable, since the files that use it are that old.
|
||||
- `clawhdf5-format` reader — errors on valid files: enum and bool datasets
|
||||
through the numeric readers; the "don't filter partial edge chunks" layout
|
||||
flag; Fletcher32 ahead of deflate (NetCDF-4's order). Unknown-message flags
|
||||
|
||||
@@ -423,13 +423,36 @@ impl Datatype {
|
||||
ensure_len(data, pos, 4)?;
|
||||
let byte_offset = LittleEndian::read_u32(&data[pos..pos + 4]) as u64;
|
||||
pos += 4;
|
||||
// v1 members can be fixed-size arrays of their
|
||||
// datatype (HDF5 before 1.4 had no array class):
|
||||
// libhdf5 wraps such a member in an array type of the
|
||||
// first `ndims` of the four stored dimensions and
|
||||
// ignores the permutation.
|
||||
let mut legacy_dims = Vec::new();
|
||||
if version == 1 {
|
||||
ensure_len(data, pos, 28)?;
|
||||
let ndims = data[pos] as usize;
|
||||
if ndims > 4 {
|
||||
return Err(FormatError::InvalidDatatypeVersion {
|
||||
class: class_id,
|
||||
version,
|
||||
});
|
||||
}
|
||||
for i in 0..ndims {
|
||||
let at = pos + 12 + 4 * i;
|
||||
legacy_dims.push(LittleEndian::read_u32(&data[at..at + 4]));
|
||||
}
|
||||
pos += 28;
|
||||
}
|
||||
let (member_dt, consumed) =
|
||||
let (mut member_dt, consumed) =
|
||||
Self::parse_with_depth(&data[pos..], depth + 1)?;
|
||||
pos += consumed;
|
||||
if !legacy_dims.is_empty() {
|
||||
member_dt = Datatype::Array {
|
||||
base_type: Box::new(member_dt),
|
||||
dimensions: legacy_dims,
|
||||
};
|
||||
}
|
||||
members.push(CompoundMember {
|
||||
name,
|
||||
byte_offset,
|
||||
@@ -1318,6 +1341,66 @@ mod tests {
|
||||
assert_xyid_compound(dt);
|
||||
}
|
||||
|
||||
/// A v1 compound member with legacy array dimensions (HDF5 before 1.4,
|
||||
/// e.g. `tarrold.h5`): `{ i: i16, f: f32[2][3] }`. The member must become
|
||||
/// an array type, not a scalar at the member's offset.
|
||||
#[test]
|
||||
fn test_compound_v1_legacy_array_member() {
|
||||
let i16le: [u8; 12] = [
|
||||
0x10, 0x08, 0x00, 0x00, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x10, 0x00,
|
||||
];
|
||||
let f32le: [u8; 20] = [
|
||||
0x11, 0x20, 0x1f, 0x00, 0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x20, 0x00, 0x17, 0x08,
|
||||
0x00, 0x17, 0x7f, 0x00, 0x00, 0x00,
|
||||
];
|
||||
let mut b = vec![0x16, 0x02, 0x00, 0x00, 28, 0x00, 0x00, 0x00];
|
||||
for (name, offset, ndims, dims, dt) in [
|
||||
(&b"i"[..], 0u32, 0u8, [0u32; 4], &i16le[..]),
|
||||
(&b"f"[..], 4, 2, [2, 3, 0, 0], &f32le[..]),
|
||||
] {
|
||||
let mut padded = name.to_vec();
|
||||
padded.resize((name.len() + 1 + 7) & !7, 0);
|
||||
b.extend_from_slice(&padded);
|
||||
b.extend_from_slice(&offset.to_le_bytes());
|
||||
b.extend_from_slice(&[ndims, 0, 0, 0]);
|
||||
b.extend_from_slice(&[0, 1, 2, 3]); // dimension permutation
|
||||
b.extend_from_slice(&[0; 4]);
|
||||
for d in dims {
|
||||
b.extend_from_slice(&d.to_le_bytes());
|
||||
}
|
||||
b.extend_from_slice(dt);
|
||||
}
|
||||
let (dt, consumed) = Datatype::parse(&b).unwrap();
|
||||
assert_eq!(consumed, b.len());
|
||||
let Datatype::Compound { size, members } = dt else {
|
||||
panic!("expected Compound, got {dt:?}");
|
||||
};
|
||||
assert_eq!(size, 28);
|
||||
assert!(matches!(
|
||||
members[0].datatype,
|
||||
Datatype::FixedPoint { size: 2, .. }
|
||||
));
|
||||
match &members[1].datatype {
|
||||
Datatype::Array {
|
||||
base_type,
|
||||
dimensions,
|
||||
} => {
|
||||
assert_eq!(dimensions, &[2, 3]);
|
||||
assert!(matches!(
|
||||
**base_type,
|
||||
Datatype::FloatingPoint { size: 4, .. }
|
||||
));
|
||||
}
|
||||
other => panic!("expected an array member, got {other:?}"),
|
||||
}
|
||||
assert_eq!(members[1].datatype.type_size(), 24);
|
||||
|
||||
// More than four legacy dimensions is not a valid message.
|
||||
let mut bad = b.clone();
|
||||
bad[8 + 8 + 4] = 5; // first member's dimensionality
|
||||
assert!(Datatype::parse(&bad).is_err());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_compound_v2_padded_names_no_array_fields() {
|
||||
// v2 = v1 without the 28 bytes of per-member array fields; names are
|
||||
|
||||
@@ -9,3 +9,4 @@ write these structures, so they are kept as files.
|
||||
|---|---|---|
|
||||
| `deflate.h5` | `test/testfiles/deflate.h5` | Data Layout message v1, chunked + deflate (v1 B-tree index) |
|
||||
| `h5ex_g_iterate.h5` | `HDF5Examples/C/H5G/h5ex_g_iterate.h5` | Data Layout message v2, contiguous; an unallocated dataset |
|
||||
| `tarrold.h5` | `test/testfiles/tarrold.h5` | Compound datatype v1 members with legacy array dimensions |
|
||||
|
||||
Binary file not shown.
@@ -9,7 +9,7 @@
|
||||
|
||||
use std::process::Command;
|
||||
|
||||
use clawhdf5::File;
|
||||
use clawhdf5::{DType, File};
|
||||
use clawhdf5_format::selection::Selection;
|
||||
|
||||
const FIXTURES: &str = concat!(
|
||||
@@ -73,6 +73,29 @@ fn layout_v2_contiguous() {
|
||||
assert_eq!(file.dataset("DS1").unwrap().read_i32().unwrap(), [0]);
|
||||
}
|
||||
|
||||
/// Compound datatype version 1 members carrying legacy array dimensions
|
||||
/// (HDF5 before 1.4 had no array class). h5py: `[('i', '<i2'), ('f', '<f4',
|
||||
/// (4,)), ('l', '<i4', (4,)), ('d', '<f8')]`, itemsize 44.
|
||||
#[test]
|
||||
fn compound_v1_legacy_array_members() {
|
||||
let file = open("tarrold.h5");
|
||||
let ds = file.dataset("Dataset2").unwrap();
|
||||
assert_eq!(
|
||||
ds.dtype().unwrap(),
|
||||
DType::Compound(vec![
|
||||
("i".into(), DType::I16),
|
||||
("f".into(), DType::Array(Box::new(DType::F32), vec![4])),
|
||||
("l".into(), DType::Array(Box::new(DType::I32), vec![4])),
|
||||
("d".into(), DType::F64),
|
||||
])
|
||||
);
|
||||
assert_eq!(ds.shape().unwrap(), [8, 9]);
|
||||
assert_eq!(
|
||||
ds.read_selection(&Selection::All).unwrap().len(),
|
||||
8 * 9 * 44
|
||||
);
|
||||
}
|
||||
|
||||
/// Every dataset in every fixture, byte for byte against h5py.
|
||||
#[test]
|
||||
fn legacy_fixtures_match_h5py() {
|
||||
@@ -87,6 +110,7 @@ fn legacy_fixtures_match_h5py() {
|
||||
for (name, datasets) in [
|
||||
("deflate.h5", &["Dataset1"][..]),
|
||||
("h5ex_g_iterate.h5", &["DS1", "G1/DS2"][..]),
|
||||
("tarrold.h5", &["Dataset1", "Dataset2"][..]),
|
||||
] {
|
||||
let path = format!("{FIXTURES}/{name}");
|
||||
let script = format!(
|
||||
|
||||
@@ -70,6 +70,10 @@ the VDS item, which is marked.
|
||||
sweep files, `InvalidLayoutVersion`. This is the largest single gap.
|
||||
**Fixed 2026-09-25:** versions 1 and 2 are parsed (compact, contiguous,
|
||||
chunked via the v1 B-tree).
|
||||
- **Compound datatype version 1 array members** (found with the layout
|
||||
fix; pre-1.4 files such as `tarrold.h5`): **wrong data** — the legacy
|
||||
per-member dimensions were skipped, so an array member read as one scalar.
|
||||
**Fixed 2026-09-25.**
|
||||
- **Virtual datasets:**
|
||||
- **Wrong data:** unmapped regions read as 0 instead of the fill value.
|
||||
- `%b` printf-style source names are not expanded.
|
||||
|
||||
Reference in New Issue
Block a user