From 8ce6eca34da93fa28407fad7f00595d60ba03d31 Mon Sep 17 00:00:00 2001 From: osobh Date: Sat, 26 Sep 2026 08:25:05 -0500 Subject: [PATCH] feat(facade): read VL strings and VL sequences through File VL-string datasets (h5py's default str dtype) failed read_string with "type mismatch: expected String, got VariableLength". read_string now reads fixed- and variable-length strings, with h5py's values (a string ends at a NUL, a null element is ""). New: - Dataset::read_string_bytes: each VL string's exact bytes; - Dataset::read_string_selection: hyperslabs/points of either kind; - Dataset::read_vlen::() and read_vlen_selection::(): VL sequences of numbers as Vec>, T in f64/f32/i64/i32/u64, converted like the other typed readers; - File::decode_strings / decode_string_bytes / decode_vlen: VL values in compound fields and AttrValue::Raw attributes; - MmapDataset and LazyDataset: read_string for VL strings, read_string_bytes and read_vlen. tests/vl_data_interop.rs checks every path against h5py with 8- and 4-byte offsets: scalar, 1-D and 2-D, ASCII and UTF-8, empty strings, contiguous, compact, chunked with gzip and shuffle, unwritten and partly written chunks, hyperslabs, compound members, attributes, a big-endian base type, and a patched file with an embedded NUL and mis-sized heap objects. NetCDF-4 string variables read too (netCDF4-python test). Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 18 + .../clawhdf5-netcdf4/tests/interop_tests.rs | 27 ++ crates/clawhdf5/src/lazy.rs | 40 +- crates/clawhdf5/src/lib.rs | 2 + crates/clawhdf5/src/mmap_file.rs | 40 +- crates/clawhdf5/src/reader.rs | 101 +++- crates/clawhdf5/src/vlen.rs | 143 ++++++ crates/clawhdf5/tests/vl_data_interop.rs | 444 ++++++++++++++++++ docs/known-issues.md | 11 +- 9 files changed, 818 insertions(+), 8 deletions(-) create mode 100644 crates/clawhdf5/src/vlen.rs create mode 100644 crates/clawhdf5/tests/vl_data_interop.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index 40781cf..0a16e0a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,6 +30,24 @@ a VL datatype whose stored size is not 4 + offset size + 4 (libhdf5 ignores the stored size). The conformance probe resolves VL elements with `VlResolver` too; conformance unchanged at 575 of 697. +- **VL data through the facade.** VL-string datasets (h5py's default `str` + dtype) failed `read_string` with "type mismatch: expected String, got + VariableLength". `Dataset::read_string` now reads fixed- and + variable-length strings; new `read_string_bytes` (a VL string's exact + bytes, as h5py's `Dataset[()]` returns them), `read_string_selection`, + `read_vlen::()` / `read_vlen_selection::()` for VL sequences of + numbers (`T` = `f64`, `f32`, `i64`, `i32`, `u64`; converted like the + other typed readers), and `File::decode_strings` / `decode_string_bytes` + / `decode_vlen` for VL values in compound fields and `AttrValue::Raw` + attributes. `MmapDataset` and `LazyDataset` gain `read_string` for VL + strings, `read_string_bytes` and `read_vlen`. Checked against h5py with + 8- and 4-byte offsets: scalar and 1-/2-D, ASCII and UTF-8, empty strings, + contiguous, compact, chunked with gzip/shuffle, never-written and + partly written chunks, hyperslab selections, VL members of compound + datasets and attributes (`crates/clawhdf5/tests/vl_data_interop.rs`). + NetCDF-4 `string` variables now read through + `clawhdf5_netcdf4::Variable::read_string` (checked against netCDF4-python + in `crates/clawhdf5-netcdf4/tests/interop_tests.rs`). ### Plugin filters (2026-09-26) - **LZF, bitshuffle, bzip2 and Blosc read and write, in pure Rust.** Files diff --git a/crates/clawhdf5-netcdf4/tests/interop_tests.rs b/crates/clawhdf5-netcdf4/tests/interop_tests.rs index af75136..eba85ac 100644 --- a/crates/clawhdf5-netcdf4/tests/interop_tests.rs +++ b/crates/clawhdf5-netcdf4/tests/interop_tests.rs @@ -350,3 +350,30 @@ ds.close() let press_vals = press_var.read_raw_f32().unwrap(); assert_eq!(press_vals, vec![1000.0f32, 850.0, 500.0, 200.0]); } + +#[test] +fn netcdf4_python_string_variable_clawhdf5_reads() { + // NC_STRING variables are HDF5 variable-length strings, which + // `read_string` refused ("expected String, got VariableLength") until + // 2026-09-26. + skip_if_no_netcdf4!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("strings.nc"); + let path_str = path.display().to_string(); + let script = format!( + r#" +import netCDF4 as nc +import numpy as np +ds = nc.Dataset("{path_str}", "w", format="NETCDF4") +ds.createDimension("station", 4) +v = ds.createVariable("name", str, ("station",)) +v[:] = np.array(["Oslo", "", "São Paulo", "x"], dtype=object) +ds.close() +"# + ); + run_python(&script); + + let file = NetCDF4File::open(&path).unwrap(); + let names = file.variable("name").unwrap().read_string().unwrap(); + assert_eq!(names, vec!["Oslo", "", "São Paulo", "x"]); +} diff --git a/crates/clawhdf5/src/lazy.rs b/crates/clawhdf5/src/lazy.rs index d893485..a33b9bb 100644 --- a/crates/clawhdf5/src/lazy.rs +++ b/crates/clawhdf5/src/lazy.rs @@ -422,11 +422,47 @@ impl<'f, R: HDF5Read> LazyDataset<'f, R> { Ok(data_read::read_as_u64(&raw, &dt)?) } - /// Read all data as `String` values. + /// Read all data as `String` values: fixed- or variable-length strings + /// (see [`Dataset::read_string`](crate::Dataset::read_string)). pub fn read_string(&self) -> Result, Error> { let raw = self.read_raw()?; let dt = self.datatype()?; - Ok(data_read::read_as_strings(&raw, &dt)?) + crate::vlen::decode_strings( + self.file.hdf5_bytes(), + &dt, + &raw, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// Read a variable-length string dataset as the exact bytes of each + /// string (see + /// [`Dataset::read_string_bytes`](crate::Dataset::read_string_bytes)). + pub fn read_string_bytes(&self) -> Result>, Error> { + let raw = self.read_raw()?; + let dt = self.datatype()?; + crate::vlen::decode_string_bytes( + self.file.hdf5_bytes(), + &dt, + &raw, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// Read a variable-length sequence dataset as one `Vec` per element + /// (see [`Dataset::read_vlen`](crate::Dataset::read_vlen)). + pub fn read_vlen(&self) -> Result>, Error> { + let raw = self.read_raw()?; + let dt = self.datatype()?; + crate::vlen::decode_vlen( + self.file.hdf5_bytes(), + &dt, + &raw, + self.file.offset_size(), + self.file.length_size(), + ) } /// Read all attributes of this dataset. diff --git a/crates/clawhdf5/src/lib.rs b/crates/clawhdf5/src/lib.rs index 12e2f69..f8098fa 100644 --- a/crates/clawhdf5/src/lib.rs +++ b/crates/clawhdf5/src/lib.rs @@ -30,6 +30,7 @@ pub mod lazy; pub mod mmap_file; pub mod reader; pub mod types; +pub mod vlen; pub mod writer; pub use error::Error; @@ -38,6 +39,7 @@ pub use lazy::{LazyDataset, LazyFile, LazyGroup}; pub use mmap_file::{MmapDataset, MmapFile, MmapGroup}; pub use reader::{Dataset, File, Group}; pub use types::{AttrValue, DType}; +pub use vlen::VlenValue; pub use writer::FileBuilder; #[cfg(feature = "parallel")] pub use writer::{DatasetSpec, create_datasets_parallel}; diff --git a/crates/clawhdf5/src/mmap_file.rs b/crates/clawhdf5/src/mmap_file.rs index 7f544ca..76d9ea1 100644 --- a/crates/clawhdf5/src/mmap_file.rs +++ b/crates/clawhdf5/src/mmap_file.rs @@ -336,11 +336,47 @@ impl<'f> MmapDataset<'f> { Ok(data_read::read_as_u64(&raw, &dt)?) } - /// Read all data as `String` values. + /// Read all data as `String` values: fixed- or variable-length strings + /// (see [`Dataset::read_string`](crate::Dataset::read_string)). pub fn read_string(&self) -> Result, Error> { let raw = self.read_raw()?; let dt = self.datatype()?; - Ok(data_read::read_as_strings(&raw, &dt)?) + crate::vlen::decode_strings( + self.file.hdf5_bytes(), + &dt, + &raw, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// Read a variable-length string dataset as the exact bytes of each + /// string (see + /// [`Dataset::read_string_bytes`](crate::Dataset::read_string_bytes)). + pub fn read_string_bytes(&self) -> Result>, Error> { + let raw = self.read_raw()?; + let dt = self.datatype()?; + crate::vlen::decode_string_bytes( + self.file.hdf5_bytes(), + &dt, + &raw, + self.file.offset_size(), + self.file.length_size(), + ) + } + + /// Read a variable-length sequence dataset as one `Vec` per element + /// (see [`Dataset::read_vlen`](crate::Dataset::read_vlen)). + pub fn read_vlen(&self) -> Result>, Error> { + let raw = self.read_raw()?; + let dt = self.datatype()?; + crate::vlen::decode_vlen( + self.file.hdf5_bytes(), + &dt, + &raw, + self.file.offset_size(), + self.file.length_size(), + ) } /// For contiguous datasets, return a zero-copy slice into the mmap. diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 83cbfa8..f0afa6d 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -262,6 +262,56 @@ impl File { } } + /// Decode the strings in `raw`, a buffer of elements of `datatype` read + /// from this file — for instance a variable-length string field of a + /// compound ([`clawhdf5_format::data_read::read_compound_fields`]) or an + /// [`AttrValue::Raw`] attribute. Variable-length strings are resolved in + /// this file's global heap; see [`Dataset::read_string`] for the values. + pub fn decode_strings(&self, datatype: &Datatype, raw: &[u8]) -> Result, Error> { + crate::vlen::decode_strings( + self.as_bytes(), + datatype, + raw, + self.offset_size(), + self.length_size(), + ) + } + + /// Like [`decode_strings`](Self::decode_strings) for variable-length + /// strings, returning each string's exact bytes (see + /// [`Dataset::read_string_bytes`]). + pub fn decode_string_bytes( + &self, + datatype: &Datatype, + raw: &[u8], + ) -> Result>, Error> { + crate::vlen::decode_string_bytes( + self.as_bytes(), + datatype, + raw, + self.offset_size(), + self.length_size(), + ) + } + + /// Decode the variable-length sequences in `raw`, a buffer of elements + /// of the sequence type `datatype` read from this file (a compound + /// field, an [`AttrValue::Raw`] attribute, ...). See + /// [`Dataset::read_vlen`]. + pub fn decode_vlen( + &self, + datatype: &Datatype, + raw: &[u8], + ) -> Result>, Error> { + crate::vlen::decode_vlen( + self.as_bytes(), + datatype, + raw, + self.offset_size(), + self.length_size(), + ) + } + fn parse_header(&self, address: u64) -> Result { ObjectHeader::parse( self.data.as_bytes(), @@ -498,11 +548,58 @@ impl<'f> Dataset<'f> { Ok(data_read::read_as_u64(&raw, &dt)?) } - /// Read all data as `String` values. + /// Read all data as `String` values, in row-major order. + /// + /// Works for fixed-length and variable-length string datasets (h5py's + /// default `str` dtype). A variable-length string ends at its first NUL + /// and a null element (e.g. never written) is `""`, as h5py returns + /// them; bytes that are not valid UTF-8 are replaced with U+FFFD — use + /// [`read_string_bytes`](Self::read_string_bytes) for the exact bytes. pub fn read_string(&self) -> Result, Error> { let raw = self.read_raw()?; let dt = self.datatype()?; - Ok(data_read::read_as_strings(&raw, &dt)?) + self.file.decode_strings(&dt, &raw) + } + + /// Read a variable-length string dataset as the exact bytes of each + /// string (what h5py's `Dataset[()]` returns), in row-major order. + pub fn read_string_bytes(&self) -> Result>, Error> { + let raw = self.read_raw()?; + let dt = self.datatype()?; + self.file.decode_string_bytes(&dt, &raw) + } + + /// Read the selected elements of a fixed- or variable-length string + /// dataset (see [`read_string`](Self::read_string)). + pub fn read_string_selection( + &self, + selection: &clawhdf5_format::selection::Selection, + ) -> Result, Error> { + let raw = self.read_selection(selection)?; + let dt = self.datatype()?; + self.file.decode_strings(&dt, &raw) + } + + /// Read a variable-length sequence dataset (h5py + /// `vlen_dtype(np.int32)`, ...) as one `Vec` per element, in + /// row-major order. The base type must be an integer or float type; it + /// is converted to `T` as [`read_f64`](Self::read_f64) and the other + /// typed readers convert. A null element is an empty sequence. + pub fn read_vlen(&self) -> Result>, Error> { + let raw = self.read_raw()?; + let dt = self.datatype()?; + self.file.decode_vlen(&dt, &raw) + } + + /// Read the selected elements of a variable-length sequence dataset + /// (see [`read_vlen`](Self::read_vlen)). + pub fn read_vlen_selection( + &self, + selection: &clawhdf5_format::selection::Selection, + ) -> Result>, Error> { + let raw = self.read_selection(selection)?; + let dt = self.datatype()?; + self.file.decode_vlen(&dt, &raw) } // ----- Selection-based read methods ----- diff --git a/crates/clawhdf5/src/vlen.rs b/crates/clawhdf5/src/vlen.rs new file mode 100644 index 0000000..01ba02d --- /dev/null +++ b/crates/clawhdf5/src/vlen.rs @@ -0,0 +1,143 @@ +//! Variable-length data: VL strings and VL sequences of numbers. +//! +//! A variable-length element stores a reference into the file's global heap; +//! these helpers resolve the references in a buffer of raw elements (from a +//! dataset read, a selection, a compound field or an [`AttrValue::Raw`] +//! attribute) against the file they came from. +//! +//! Values match libhdf5 (and h5py): a string ends at its first NUL, a null +//! element is an empty string or sequence, and a heap object whose size +//! disagrees with its element is an error rather than a truncated value. +//! +//! [`AttrValue::Raw`]: crate::AttrValue::Raw + +use clawhdf5_format::data_read; +use clawhdf5_format::datatype::Datatype; +use clawhdf5_format::error::FormatError; +use clawhdf5_format::vl_data::{VlResolver, check_element_size}; + +use crate::error::Error; + +mod sealed { + pub trait Sealed {} +} + +/// A number type that [`Dataset::read_vlen`](crate::Dataset::read_vlen) can +/// return: the sequence's base type is converted to it as libhdf5 converts +/// numbers (the same rules as `read_f64`, `read_i64`, ...). +pub trait VlenValue: sealed::Sealed + Sized { + #[doc(hidden)] + fn decode(raw: &[u8], base: &Datatype) -> Result, FormatError>; +} + +macro_rules! vlen_value { + ($t:ty, $f:path) => { + impl sealed::Sealed for $t {} + impl VlenValue for $t { + fn decode(raw: &[u8], base: &Datatype) -> Result, FormatError> { + $f(raw, base) + } + } + }; +} + +vlen_value!(f64, data_read::read_as_f64); +vlen_value!(f32, data_read::read_as_f32); +vlen_value!(i64, data_read::read_as_i64); +vlen_value!(i32, data_read::read_as_i32); +vlen_value!(u64, data_read::read_as_u64); + +fn class_name(dt: &Datatype) -> &'static str { + match dt { + Datatype::FixedPoint { .. } => "integer", + Datatype::FloatingPoint { .. } => "float", + Datatype::Time { .. } => "time", + Datatype::String { .. } => "fixed-length string", + Datatype::BitField { .. } => "bitfield", + Datatype::Opaque { .. } => "opaque", + Datatype::Compound { .. } => "compound", + Datatype::Reference { .. } => "reference", + Datatype::Enumeration { .. } => "enum", + Datatype::VariableLength { + is_string: true, .. + } => "variable-length string", + Datatype::VariableLength { .. } => "variable-length sequence", + Datatype::Array { .. } => "array", + } +} + +/// The strings in `raw`, elements of `dt`: fixed-length strings decoded as +/// `read_string` always has, variable-length strings resolved in the heap. +pub(crate) fn decode_strings( + file_data: &[u8], + dt: &Datatype, + raw: &[u8], + offset_size: u8, + length_size: u8, +) -> Result, Error> { + match dt { + Datatype::VariableLength { + size, + is_string: true, + .. + } => { + check_element_size(*size, offset_size)?; + Ok(VlResolver::new(file_data, offset_size, length_size).strings(raw)?) + } + _ => Ok(data_read::read_as_strings(raw, dt)?), + } +} + +/// The exact bytes of the variable-length strings in `raw`. +pub(crate) fn decode_string_bytes( + file_data: &[u8], + dt: &Datatype, + raw: &[u8], + offset_size: u8, + length_size: u8, +) -> Result>, Error> { + match dt { + Datatype::VariableLength { + size, + is_string: true, + .. + } => { + check_element_size(*size, offset_size)?; + Ok(VlResolver::new(file_data, offset_size, length_size).string_bytes(raw)?) + } + other => Err(Error::Format(FormatError::TypeMismatch { + expected: "variable-length string", + actual: class_name(other), + })), + } +} + +/// The sequences in `raw`, elements of the variable-length sequence type +/// `dt`, converted to `T`. +pub(crate) fn decode_vlen( + file_data: &[u8], + dt: &Datatype, + raw: &[u8], + offset_size: u8, + length_size: u8, +) -> Result>, Error> { + let Datatype::VariableLength { + size, + is_string: false, + base_type, + .. + } = dt + else { + return Err(Error::Format(FormatError::TypeMismatch { + expected: "variable-length sequence", + actual: class_name(dt), + })); + }; + check_element_size(*size, offset_size)?; + let base_size = base_type.type_size() as usize; + VlResolver::new(file_data, offset_size, length_size) + .sequences(raw, base_size)? + .iter() + .map(|bytes| Ok(T::decode(bytes, base_type)?)) + .collect() +} diff --git a/crates/clawhdf5/tests/vl_data_interop.rs b/crates/clawhdf5/tests/vl_data_interop.rs new file mode 100644 index 0000000..63f97f9 --- /dev/null +++ b/crates/clawhdf5/tests/vl_data_interop.rs @@ -0,0 +1,444 @@ +//! Variable-length data (VL strings and VL sequences) read through the +//! facade, checked against h5py/libhdf5. +//! +//! h5py writes each file — once with the default 8-byte offsets and once +//! with 4-byte offsets and lengths (`sizeof_addr = 4`) — and prints what +//! libhdf5 reads back; `File`, `MmapFile` and `LazyFile` must return the same +//! values. Skipped when python3 with h5py is unavailable, unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`. + +// `Selection::slice(&[0..1])` is one range per dimension, not a Vec of a range. +#![allow(clippy::single_range_in_vec_init)] + +use std::collections::HashMap; +use std::path::Path; +use std::process::Command; + +use clawhdf5::{AttrValue, File, LazyFile, MmapFile, Selection}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn interop_required() -> bool { + std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1") +} + +fn python_available() -> bool { + Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .map(|o| o.status.success()) + .unwrap_or(false) +} + +macro_rules! skip_if_no_python { + () => { + if !python_available() { + assert!( + !interop_required(), + "CLAWHDF5_REQUIRE_INTEROP=1 but python3 with h5py is not available" + ); + eprintln!("SKIP: python3 with h5py not available"); + return; + } + }; +} + +/// Run `script` and return its stdout as `key -> value`, one +/// `keyvalue` line per key. +fn run_python(script: &str) -> HashMap { + let output = Command::new(python()) + .args(["-c", script]) + .output() + .expect("failed to run python"); + assert!( + output.status.success(), + "python failed:\n{}", + String::from_utf8_lossy(&output.stderr) + ); + String::from_utf8_lossy(&output.stdout) + .lines() + .filter_map(|line| { + let (k, v) = line.split_once('\t')?; + Some((k.to_string(), v.to_string())) + }) + .collect() +} + +/// `hex,hex,...` -> the strings' bytes. +fn parse_strings(v: &str) -> Vec> { + v.split(',') + .map(|h| { + (0..h.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&h[i..i + 2], 16).unwrap()) + .collect() + }) + .collect() +} + +/// `1 2 3|| -5` -> sequences. +fn parse_seqs(v: &str) -> Vec> { + v.split('|') + .map(|s| s.split_whitespace().map(|x| x.parse().unwrap()).collect()) + .collect() +} + +fn utf8(bytes: &[Vec]) -> Vec { + bytes + .iter() + .map(|b| String::from_utf8(b.clone()).unwrap()) + .collect() +} + +/// Writes `vl8.h5` (8-byte offsets) and `vl4.h5` (4-byte offsets and +/// lengths) into `dir` and prints h5py's reading of both. +const SCRIPT: &str = r#" +import sys, h5py, numpy as np +d = sys.argv[1] +S = h5py.string_dtype('utf-8'); A = h5py.string_dtype('ascii') +def make(path, sizes): + if sizes: + fcpl = h5py.h5p.create(h5py.h5p.FILE_CREATE); fcpl.set_sizes(*sizes) + f = h5py.File(h5py.h5f.create(path.encode(), h5py.h5f.ACC_TRUNC, fcpl=fcpl)) + else: + f = h5py.File(path, 'w') + f.create_dataset('scalar_utf8', data='héllo', dtype=S) + f.create_dataset('scalar_ascii', data=b'hello', dtype=A) + f.create_dataset('d1', data=np.array(['a', '', 'ccc', 'δδ'], dtype=object), dtype=S) + f.create_dataset('d2', data=np.array([['x', 'yy', 'zzz'], ['', 'w', 'vv']], dtype=object), dtype=S) + f.create_dataset('chunked', data=np.array(['s%d' % i * (i % 5) for i in range(100)], dtype=object), + dtype=S, chunks=(7,), compression='gzip') + f.create_dataset('chunked2d', data=np.array([['r%dc%d' % (r, c) for c in range(9)] for r in range(11)], dtype=object), + dtype=S, chunks=(4, 4), compression='gzip', shuffle=True) + f.create_dataset('unwritten', shape=(5,), dtype=S, chunks=(2,)) + p = f.create_dataset('partial', shape=(6,), dtype=S, chunks=(2,)); p[0] = 'first'; p[5] = 'last' + f.create_dataset('contig_empty', shape=(3,), dtype=S) + dcpl = h5py.h5p.create(h5py.h5p.DATASET_CREATE); dcpl.set_layout(h5py.h5d.COMPACT) + f.create_dataset('compact', data=np.array(['c1', '', 'c3'], dtype=object), dtype=S, dcpl=dcpl) + assert f['compact'].id.get_create_plist().get_layout() == h5py.h5d.COMPACT + f.attrs['vlattr'] = 'attr-value' + f.attrs.create('vlattr_arr', np.array(['p', 'qq', ''], dtype=object), dtype=S) + ct = np.dtype([('id', 'u2'))) + v[0] = [1, 65535]; v[1] = [300] + f.attrs.create('vlen_attr', np.array([np.array([1, 2], dtype=' HashMap { + let script = format!( + "import sys; sys.argv = ['x', {:?}]\n{SCRIPT}", + dir.display().to_string() + ); + run_python(&script) +} + +const STRING_DATASETS: [&str; 10] = [ + "compact", + "scalar_utf8", + "scalar_ascii", + "d1", + "d2", + "chunked", + "chunked2d", + "unwritten", + "partial", + "contig_empty", +]; + +#[test] +fn vl_string_datasets_read_like_h5py() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let expected = make_files(dir.path()); + for tag in ["8", "4"] { + let path = dir.path().join(format!("vl{tag}.h5")); + let file = File::open(&path).unwrap(); + let mmap = MmapFile::open(&path).unwrap(); + let lazy = LazyFile::open_mmap(&path).unwrap(); + for name in STRING_DATASETS { + let want = parse_strings(&expected[&format!("{tag}:{name}")]); + let ctx = format!("vl{tag}.h5 {name}"); + let ds = file.dataset(name).unwrap(); + assert_eq!(ds.read_string_bytes().unwrap(), want, "{ctx}"); + assert_eq!(ds.read_string().unwrap(), utf8(&want), "{ctx}"); + let m = mmap.dataset(name).unwrap(); + assert_eq!(m.read_string_bytes().unwrap(), want, "{ctx} (mmap)"); + assert_eq!(m.read_string().unwrap(), utf8(&want), "{ctx} (mmap)"); + let l = lazy.dataset(name).unwrap(); + assert_eq!(l.read_string_bytes().unwrap(), want, "{ctx} (lazy)"); + assert_eq!(l.read_string().unwrap(), utf8(&want), "{ctx} (lazy)"); + } + } +} + +#[test] +fn vl_string_selections_read_like_h5py() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let expected = make_files(dir.path()); + let hyperslab = |start: &[u64], stride: &[u64], count: &[u64]| Selection::Hyperslab { + start: start.to_vec(), + stride: stride.to_vec(), + count: count.to_vec(), + block: vec![1; start.len()], + }; + let cases = [ + ("d2", "d2[1,1:3]", Selection::slice(&[1..2, 1..3])), + ("chunked", "chunked[5:60:3]", hyperslab(&[5], &[3], &[19])), + ( + "chunked2d", + "chunked2d[2:9:2,3:8]", + hyperslab(&[2, 3], &[2, 1], &[4, 5]), + ), + ]; + for tag in ["8", "4"] { + let file = File::open(dir.path().join(format!("vl{tag}.h5"))).unwrap(); + for (name, key, sel) in &cases { + let want = utf8(&parse_strings(&expected[&format!("{tag}:{key}")])); + let got = file + .dataset(name) + .unwrap() + .read_string_selection(sel) + .unwrap(); + assert_eq!(got, want, "vl{tag}.h5 {key}"); + } + // A selection of VL integers is not strings. + assert!( + file.dataset("vlen_i4") + .unwrap() + .read_string_selection(&Selection::slice(&[0..1])) + .is_err() + ); + } +} + +#[test] +fn vl_values_in_compounds_and_attributes_read_like_h5py() { + // With 4-byte offsets these failed with GlobalHeapObjectNotFound or came + // back as `AttrValue::Raw`: the VL type claimed 16-byte elements and the + // global heap was read without the padding libhdf5 puts after its + // headers. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let expected = make_files(dir.path()); + for tag in ["8", "4"] { + let file = File::open(dir.path().join(format!("vl{tag}.h5"))).unwrap(); + let want = |key: &str| utf8(&parse_strings(&expected[&format!("{tag}:{key}")])); + + let attrs = file.root().attrs().unwrap(); + match &attrs["vlattr"] { + AttrValue::String(s) => assert_eq!(*s, want("vlattr")[0], "vl{tag}.h5"), + other => panic!("vl{tag}.h5 vlattr: {other:?}"), + } + match &attrs["vlattr_arr"] { + AttrValue::StringArray(s) => assert_eq!(*s, want("vlattr_arr"), "vl{tag}.h5"), + other => panic!("vl{tag}.h5 vlattr_arr: {other:?}"), + } + + // Compound with a VL string member: dataset and attribute. + let ds = file.dataset("compound").unwrap(); + let dt = ds.raw_datatype().unwrap(); + let raw = ds.read_selection(&Selection::All).unwrap(); + let fields = clawhdf5_format::data_read::read_compound_fields(&raw, &dt).unwrap(); + let name = fields.iter().find(|f| f.name == "name").unwrap(); + assert_eq!( + file.decode_strings(&name.datatype, &name.raw_data).unwrap(), + want("compound.name"), + "vl{tag}.h5 compound" + ); + let id = fields.iter().find(|f| f.name == "id").unwrap(); + assert_eq!( + clawhdf5_format::data_read::read_as_i64(&id.raw_data, &id.datatype).unwrap(), + vec![1, 2, 3] + ); + let v = fields.iter().find(|f| f.name == "v").unwrap(); + assert_eq!( + clawhdf5_format::data_read::read_as_f64(&v.raw_data, &v.datatype).unwrap(), + vec![0.5, 1.5, 2.5] + ); + + let AttrValue::Raw { datatype, data, .. } = &attrs["compound_attr"] else { + panic!("compound attribute is Raw"); + }; + let fields = clawhdf5_format::data_read::read_compound_fields(data, datatype).unwrap(); + let name = fields.iter().find(|f| f.name == "name").unwrap(); + assert_eq!( + file.decode_strings(&name.datatype, &name.raw_data).unwrap(), + want("compound_attr.name"), + "vl{tag}.h5 compound attribute" + ); + + // A VL sequence attribute. + let AttrValue::Raw { datatype, data, .. } = &attrs["vlen_attr"] else { + panic!("vlen attribute is Raw"); + }; + let got: Vec> = file.decode_vlen(datatype, data).unwrap(); + let want_seqs = parse_seqs(&expected[&format!("{tag}:vlen_attr")]); + assert_eq!( + got, + want_seqs + .iter() + .map(|s| s.iter().map(|&x| x as i64).collect::>()) + .collect::>(), + "vl{tag}.h5 vlen_attr" + ); + } +} + +#[test] +fn vl_sequence_datasets_read_like_h5py() { + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let expected = make_files(dir.path()); + for tag in ["8", "4"] { + let path = dir.path().join(format!("vl{tag}.h5")); + let file = File::open(&path).unwrap(); + let seqs = |key: &str| parse_seqs(&expected[&format!("{tag}:{key}")]); + + let i4 = file.dataset("vlen_i4").unwrap(); + let want: Vec> = seqs("vlen_i4") + .iter() + .map(|s| s.iter().map(|&x| x as i32).collect()) + .collect(); + assert_eq!(i4.read_vlen::().unwrap(), want, "vl{tag}.h5 vlen_i4"); + let as_f64: Vec> = i4.read_vlen().unwrap(); + assert_eq!(as_f64, seqs("vlen_i4")); + + let f8 = file.dataset("vlen_f8").unwrap(); + assert_eq!( + f8.read_vlen::().unwrap(), + seqs("vlen_f8"), + "vl{tag}.h5" + ); + assert_eq!( + f8.read_vlen_selection::(&Selection::slice(&[1..2, 0..2])) + .unwrap(), + seqs("vlen_f8[1,:]"), + "vl{tag}.h5 vlen_f8[1,:]" + ); + + // h5py returns big-endian VL elements byte-swapped (an h5py bug, see + // CONFORMANCE.md); the values written are [1, 65535] and [300]. + assert_eq!( + file.dataset("vlen_u2_be") + .unwrap() + .read_vlen::() + .unwrap(), + vec![vec![1, 65535], vec![300]] + ); + + let mmap = MmapFile::open(&path).unwrap(); + assert_eq!( + mmap.dataset("vlen_f8").unwrap().read_vlen::().unwrap(), + seqs("vlen_f8") + ); + let lazy = LazyFile::open_mmap(&path).unwrap(); + assert_eq!( + lazy.dataset("vlen_f8").unwrap().read_vlen::().unwrap(), + seqs("vlen_f8") + ); + + // Wrong kind of data is an error, not a value. + assert!(i4.read_string().is_err()); + assert!(i4.read_string_bytes().is_err()); + assert!(file.dataset("d1").unwrap().read_vlen::().is_err()); + } +} + +#[test] +fn vl_strings_end_at_nul_and_mis_sized_elements_fail_like_h5py() { + // h5py cannot write a VL string with a NUL in it, so the file is patched: + // one string gets an embedded NUL, and two elements get a length that + // disagrees with their heap object. libhdf5 returns the string up to the + // NUL and refuses the others ("Expected global heap object size does + // not match"); we used to return the NUL and a truncated string. + skip_if_no_python!(); + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("patched.h5"); + let script = format!( + r#" +import struct, h5py, numpy as np +path = {path:?} +with h5py.File(path, 'w') as f: + f.create_dataset('d', data=np.array(['aXb', 'cdefgh', 'ij', 'ok'], dtype=object), + dtype=h5py.string_dtype()) + s = f.create_dataset('seq', shape=(2,), dtype=h5py.vlen_dtype(np.dtype(' 3 +struct.pack_into(' 9 +struct.pack_into(' 2 +open(path, 'wb').write(bytes(b)) +with h5py.File(path, 'r') as f: + for i in range(4): + try: + print('d%d\t%s' % (i, f['d'][i].hex())) + except OSError as e: + print('d%d\terror' % i) + for i in range(2): + try: + print('seq%d\t%s' % (i, ' '.join(str(x) for x in f['seq'][i]))) + except OSError as e: + print('seq%d\terror' % i) +"#, + path = path.display().to_string() + ); + let expected = run_python(&script); + assert_eq!(expected["d0"], "61", "h5py cuts 'a\\0b' at the NUL"); + assert_eq!(expected["d1"], "error"); + assert_eq!(expected["d2"], "error"); + assert_eq!(expected["d3"], "6f6b"); + assert_eq!(expected["seq0"], "error"); + assert_eq!(expected["seq1"], "4"); + + let file = File::open(&path).unwrap(); + let d = file.dataset("d").unwrap(); + let one = |i: u64| d.read_string_selection(&Selection::slice(&[i..i + 1])); + assert_eq!(one(0).unwrap(), vec!["a"]); + assert!(one(1).is_err()); + assert!(one(2).is_err()); + assert_eq!(one(3).unwrap(), vec!["ok"]); + assert!(d.read_string().is_err()); + let seq = file.dataset("seq").unwrap(); + let one = |i: u64| seq.read_vlen_selection::(&Selection::slice(&[i..i + 1])); + assert!(one(0).is_err()); + assert_eq!(one(1).unwrap(), vec![vec![4]]); +} diff --git a/docs/known-issues.md b/docs/known-issues.md index b9a19b6..4ea8307 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -140,7 +140,13 @@ fill-value item that did is fixed). is left out of `attrs()` (reported by `attrs_with_errors()`) instead of failing the others. - **Other readers:** - - VL-string datasets are not readable through `File`. + - VL-string datasets are not readable through `File`. **Fixed + 2026-09-26:** `read_string` reads them (also `read_string_bytes`, + `read_string_selection`, and on `MmapFile`/`LazyFile`), with h5py's + values: strings end at a NUL, null elements are `""`; VL sequences of + numbers read with `read_vlen::()`, and VL values inside compounds or + `AttrValue::Raw` attributes decode with `File::decode_strings` / + `File::decode_vlen` (`crates/clawhdf5/tests/vl_data_interop.rs`). - Variable-length values inside a compound (and VL-string attributes) in a file with 4-byte offsets (`sizeof_addr = 4`) fail with `GlobalHeapObjectNotFound` or come back as `Raw`: these paths assume @@ -505,7 +511,8 @@ which is what libhdf5 itself writes. followed (no file system). - Variable-length string datasets are read by decoding `read_selection`'s bytes with `clawhdf5_format::vl_data` in the wasm crate; `File` itself still - cannot (see the audit gaps above). + cannot (see the audit gaps above). (`File` can since 2026-09-26; the wasm + crate still decodes them itself.) ## The Node.js package (`packages/clawhdf5-node`) does not work