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.

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 {

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)));
+}