diff --git a/CHANGELOG.md b/CHANGELOG.md index 3092ae5..080045d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,11 @@ `h5rs check` passes and `h5rs dump` equals h5dump (`crates/clawhdf5/tests/writer_groups_interop.rs`, `crates/clawhdf5-tools/tests/h5rs_interop.rs`). +- **A group attribute set twice read back as its first value.** Setting a + group (or root) attribute again wrote a second attribute message with the + same name, and h5py returned the first value. The later value now replaces + the earlier one, as `attrs[name] = v` does in h5py — also when a group is + merged from two builders. - **Non-ASCII link names were marked ASCII.** A group or dataset name such as `größe` was written with the ASCII character set flag (h5py reported `cset` 0 for it); it is now flagged UTF-8, as h5py writes it. diff --git a/crates/clawhdf5-format/src/writer_tree.rs b/crates/clawhdf5-format/src/writer_tree.rs index 284867e..724ba70 100644 --- a/crates/clawhdf5-format/src/writer_tree.rs +++ b/crates/clawhdf5-format/src/writer_tree.rs @@ -229,14 +229,14 @@ impl Builder { /// Merge a builder's attributes, setting and items into group `idx`. fn merge_into(&mut self, idx: usize, gb: GroupBuilder) -> Result<(), FormatError> { + // An attribute set again (by this builder or a merged one) takes the + // new value, as assigning `attrs[name]` in h5py does. for (name, value) in gb.attrs { - if self.groups[idx].attrs.iter().any(|(n, _)| *n == name) { - return Err(err(format!( - "attribute {name:?} set twice on {}", - self.groups[idx].path - ))); + let attrs = &mut self.groups[idx].attrs; + match attrs.iter_mut().find(|(n, _)| *n == name) { + Some(slot) => slot.1 = value, + None => attrs.push((name, value)), } - self.groups[idx].attrs.push((name, value)); } if let Some(t) = gb.track_order { match self.groups[idx].track_order { diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index fb7aa23..ba2d121 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -610,3 +610,31 @@ fn non_ascii_names_are_utf8() { let f = File::open(&path).unwrap(); assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]); } + +#[test] +fn a_group_attribute_set_again_takes_the_new_value() { + skip_if_no_python!(); + // Setting a group attribute twice wrote two attribute messages with one + // name. Now the later value replaces the earlier, as `attrs[name] = v` + // does in h5py — also across a group merged from two builders. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.set_attr("v", AttrValue::I64(1)); + b.set_attr("v", AttrValue::I64(2)); + let mut g = b.create_group("g"); + g.set_attr("w", AttrValue::I64(1)); + b.add_group(g.finish()); + let mut g = b.create_group("g"); + g.set_attr("w", AttrValue::String("two".into())); + b.add_group(g.finish()); + let path = write(&dir, "attrs.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 print(json.dumps([list(f.attrs), int(f.attrs['v']), list(f['g'].attrs),\n\ + \x20 f['g'].attrs['w'].decode()]))", + ); + assert_eq!(out, r#"[["v"], 2, ["w"], "two"]"#); + let f = File::open(&path).unwrap(); + assert!(matches!(f.root().attrs().unwrap()["v"], AttrValue::I64(2))); +}