fix(format): a group attribute set again replaces the earlier value

Setting a group or root attribute twice wrote two attribute messages with
the same name, and h5py read back the first value: set_attr("w", 1) then
set_attr("w", "two") read as 1. The later value now replaces the earlier
one, as `attrs[name] = v` does in h5py, including when a group is merged
from two builders.

Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
2026-09-26 08:35:16 -05:00
co-authored by Claude Opus 5.5
parent bd36fe883b
commit b0a1e4f9a6
3 changed files with 39 additions and 6 deletions
+5
View File
@@ -41,6 +41,11 @@
`h5rs check` passes and `h5rs dump` equals h5dump `h5rs check` passes and `h5rs dump` equals h5dump
(`crates/clawhdf5/tests/writer_groups_interop.rs`, (`crates/clawhdf5/tests/writer_groups_interop.rs`,
`crates/clawhdf5-tools/tests/h5rs_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 - **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 `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. `cset` 0 for it); it is now flagged UTF-8, as h5py writes it.
+6 -6
View File
@@ -229,14 +229,14 @@ impl Builder {
/// Merge a builder's attributes, setting and items into group `idx`. /// Merge a builder's attributes, setting and items into group `idx`.
fn merge_into(&mut self, idx: usize, gb: GroupBuilder) -> Result<(), FormatError> { 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 { for (name, value) in gb.attrs {
if self.groups[idx].attrs.iter().any(|(n, _)| *n == name) { let attrs = &mut self.groups[idx].attrs;
return Err(err(format!( match attrs.iter_mut().find(|(n, _)| *n == name) {
"attribute {name:?} set twice on {}", Some(slot) => slot.1 = value,
self.groups[idx].path None => attrs.push((name, value)),
)));
} }
self.groups[idx].attrs.push((name, value));
} }
if let Some(t) = gb.track_order { if let Some(t) = gb.track_order {
match self.groups[idx].track_order { match self.groups[idx].track_order {
@@ -610,3 +610,31 @@ fn non_ascii_names_are_utf8() {
let f = File::open(&path).unwrap(); let f = File::open(&path).unwrap();
assert_eq!(f.dataset("größe/wert").unwrap().read_i32().unwrap(), [1]); 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)));
}