fix(format): a dataset attribute set again replaces the earlier value
b0a1e4f fixed this for group and root attributes only. Setting a dataset
attribute twice still wrote two attribute messages with one name, and h5py
read back the first value: set_attr("a", 1) then set_attr("a", 2) read as
1, and list(attrs) was ["a", "a"]. DatasetBuilder::set_attr now replaces
the earlier value, compact or dense. Likewise, a hand-set attribute named
like a provenance attribute (_provenance_sha256, ...) is replaced by the
computed one instead of being written next to it and read first.
Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
@@ -1355,7 +1355,10 @@ fn flatten_ds(db: DatasetBuilder, refcount: u32) -> Result<DsFlat, FormatError>
|
|||||||
timestamp: prov.timestamp.clone(),
|
timestamp: prov.timestamp.clone(),
|
||||||
source: prov.source.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)?;
|
let fill_message = fill_value_message(db.fill_time, db.fill_value.as_deref(), &dt)?;
|
||||||
Ok(DsFlat {
|
Ok(DsFlat {
|
||||||
|
|||||||
@@ -695,8 +695,13 @@ impl DatasetBuilder {
|
|||||||
self
|
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 {
|
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
|
self
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@@ -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]);
|
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
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user