diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 0b318a9..11345a1 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -1355,7 +1355,10 @@ fn flatten_ds(db: DatasetBuilder, refcount: u32) -> Result timestamp: prov.timestamp.clone(), source: prov.source.clone(), }; - attrs.extend(p.build_attrs(&raw)); + // The provenance attributes replace any the caller set by hand. + let prov = p.build_attrs(&raw); + attrs.retain(|a| prov.iter().all(|b| b.name != a.name)); + attrs.extend(prov); } let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?; Ok(DsFlat { diff --git a/crates/clawhdf5-format/src/type_builders.rs b/crates/clawhdf5-format/src/type_builders.rs index fa64465..ce43f64 100644 --- a/crates/clawhdf5-format/src/type_builders.rs +++ b/crates/clawhdf5-format/src/type_builders.rs @@ -695,8 +695,13 @@ impl DatasetBuilder { self } + /// Set attribute `name`. Setting it again replaces the earlier value, + /// as `attrs[name] = v` does in h5py. pub fn set_attr(&mut self, name: &str, value: AttrValue) -> &mut Self { - self.attrs.push((name.to_string(), value)); + match self.attrs.iter_mut().find(|(n, _)| n == name) { + Some(slot) => slot.1 = value, + None => self.attrs.push((name.to_string(), value)), + } self } diff --git a/crates/clawhdf5/tests/writer_groups_interop.rs b/crates/clawhdf5/tests/writer_groups_interop.rs index e8488c9..97e3544 100644 --- a/crates/clawhdf5/tests/writer_groups_interop.rs +++ b/crates/clawhdf5/tests/writer_groups_interop.rs @@ -876,3 +876,65 @@ fn chained_hard_links_resolve_in_linear_time() { assert_eq!(f.dataset("g/s59/s0/v").unwrap().read_i32().unwrap(), [5]); } } + +#[test] +fn a_dataset_attribute_set_again_takes_the_new_value() { + skip_if_no_python!(); + // Setting a dataset attribute twice wrote two attribute messages with + // one name, and h5py read back the first value. Also with dense + // attribute storage (more than 8). + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("x") + .with_f64_data(&[1.0]) + .set_attr("a", AttrValue::I64(1)) + .set_attr("a", AttrValue::I64(2)); + let d = b.create_dataset("dense"); + d.with_i32_data(&[1]); + for i in 0..12 { + d.set_attr(&format!("k{i:02}"), AttrValue::I64(i)); + } + d.set_attr("k03", AttrValue::String("three".into())); + let path = write(&dir, "ds_attrs.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 a, d = f['x'].attrs, f['dense'].attrs\n\ + \x20 print(json.dumps([list(a), int(a['a']), len(d), d['k03'].decode(), int(d['k04'])]))", + ); + assert_eq!(out, r#"[["a"], 2, 12, "three", 4]"#); + let f = File::open(&path).unwrap(); + assert!(matches!( + f.dataset("x").unwrap().attrs().unwrap()["a"], + AttrValue::I64(2) + )); +} + +#[cfg(feature = "provenance")] +#[test] +fn provenance_attributes_replace_ones_set_by_hand() { + skip_if_no_python!(); + // A hand-set attribute with a provenance attribute's name was written + // next to the computed one, and h5py read the hand-set value. + let dir = tempfile::tempdir().unwrap(); + let mut b = FileBuilder::new(); + b.create_dataset("p") + .with_i32_data(&[1, 2]) + .with_provenance("me", "2026-09-26T00:00:00Z", None) + .set_attr("_provenance_sha256", AttrValue::String("forged".into())); + let path = write(&dir, "prov.h5", b); + let out = h5py( + &path, + "with h5py.File(path, 'r') as f:\n\ + \x20 a = f['p'].attrs\n\ + \x20 h = a['_provenance_sha256']\n\ + \x20 h = h.decode() if isinstance(h, bytes) else h\n\ + \x20 print(json.dumps([list(a).count('_provenance_sha256'), h != 'forged']))", + ); + assert_eq!(out, "[1, true]"); + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("p").unwrap().verify_provenance().unwrap(), + clawhdf5_format::provenance::VerifyResult::Ok + ); +}