clawhdf5: set_attr adds the Attribute Info message a version-2 header needs
libhdf5 counts a version-2 object header's attributes through its Attribute Info message (0x15) and reports none when the header has none. set_attr gave v110/latest groups, the root group and datasets without attributes an attribute message only, so h5py listed the attribute but len(obj.attrs) and H5Oget_info's num_attrs said 0, and stayed wrong after h5py r+ added more. Like H5O__attr_create, the edit now adds the message when a version-2 header lacks it, in the same planned edit: version 0, the header's creation-order track/index flags, maximum creation index 0, undefined fractal heap and B-tree addresses, message flag DONTSHARE — byte for byte what libhdf5 writes. It goes before the attribute (libhdf5's order) when free space holds both, else after it, so a continuation chunk made for the attribute also takes it. Test: edit_interop attribute_count_in_version_2_headers — v110 and latest files, attributes set on the root group, groups and datasets with and without existing attributes: h5py's len/num_attrs/list/values, h5dump -A and our reader agree, also after h5py r+ adds attributes up to and past the compact limit. Fails on the previous editor (h5py len 0). Co-Authored-By: Claude Opus 5.5 (1M context) <[email protected]>
This commit is contained in:
osobh
+5
-1
@@ -31,7 +31,11 @@
|
||||
- `resize`: grow a chunked dataset up to its maximum dimensions (h5py's
|
||||
`Dataset.resize`).
|
||||
- `set_attr`: add or replace an attribute in an object header, in free
|
||||
space or in a new continuation chunk at the end of the file.
|
||||
space or in a new continuation chunk at the end of the file. A
|
||||
version-2 header (h5py `libver='v110'` and later) without an Attribute
|
||||
Info message gets one, as libhdf5's `H5O__attr_create` adds it: libhdf5
|
||||
counts such a header's attributes through that message, and without it
|
||||
h5py reported `len(obj.attrs) == 0` while listing them.
|
||||
- Each edit is planned in memory and refused as a whole
|
||||
(`Error::Unsupported`, file untouched) when any part is not supported:
|
||||
new chunks in a version-2 B-tree index (two or more unlimited
|
||||
|
||||
@@ -1315,3 +1315,90 @@ fn optional_filters_that_fail_are_skipped() {
|
||||
assert!(o.status.success(), "h5rs check --data {p}:\n{}", text(&o));
|
||||
}
|
||||
}
|
||||
|
||||
/// libhdf5 counts a version-2 object header's attributes through its
|
||||
/// Attribute Info message and reports none without one. The editor adds
|
||||
/// that message, as `H5O__attr_create` does, when it gives a version-2
|
||||
/// header (h5py `libver='v110'`/`'latest'`) its first attribute: afterwards
|
||||
/// h5py lists, counts and reads every attribute, new and existing, h5dump
|
||||
/// agrees, and h5py `r+` can add more (past the compact limit, into dense
|
||||
/// storage) with the count still right.
|
||||
#[test]
|
||||
fn attribute_count_in_version_2_headers() {
|
||||
if !tools_ok() {
|
||||
return;
|
||||
}
|
||||
for (lv, dump) in [("'v110'", true), ("'latest'", false)] {
|
||||
let dir = tmpdir();
|
||||
let path = dir.path().join("acount.h5");
|
||||
let p = path.to_str().unwrap();
|
||||
py(&format!(
|
||||
"import h5py, numpy as np\n\
|
||||
with h5py.File({p:?}, 'w', libver={lv}) as f:\n\
|
||||
\x20 f.create_group('g')\n\
|
||||
\x20 f.create_group('has').attrs['old'] = 7\n\
|
||||
\x20 f.create_dataset('d', data=np.arange(3, dtype='<i4'))\n\
|
||||
\x20 f.create_dataset('e', data=np.arange(3, dtype='<i4')).attrs['old'] = 7\n"
|
||||
));
|
||||
let mut ed = FileEditor::open(&path).unwrap();
|
||||
for obj in ["/", "g", "has", "d", "e"] {
|
||||
for i in 0..3 {
|
||||
ed.set_attr(obj, &format!("a{i}"), &AttrValue::I64(i))
|
||||
.unwrap();
|
||||
}
|
||||
// Replacing one keeps the count.
|
||||
ed.set_attr(obj, "a1", &AttrValue::F64(1.5)).unwrap();
|
||||
}
|
||||
drop(ed);
|
||||
check_tools(&path, dump);
|
||||
let check = |extra: usize| {
|
||||
py(&format!(
|
||||
"import h5py\n\
|
||||
f = h5py.File({p:?}, 'r')\n\
|
||||
for o in ['/', 'g', 'has', 'd', 'e']:\n\
|
||||
\x20 a = f[o].attrs\n\
|
||||
\x20 want = {{'a0': 0, 'a1': 1.5, 'a2': 2}}\n\
|
||||
\x20 if o in ('has', 'e'): want['old'] = 7\n\
|
||||
\x20 for i in range({extra}): want[f'h{{i}}'] = i\n\
|
||||
\x20 assert len(a) == len(want), (o, len(a), list(a))\n\
|
||||
\x20 assert h5py.h5o.get_info(f[o].id).num_attrs == len(want), o\n\
|
||||
\x20 assert sorted(a) == sorted(want), (o, list(a))\n\
|
||||
\x20 assert {{k: a[k] for k in a}} == want, (o, dict(a))\n"
|
||||
));
|
||||
let f = File::open(&path).unwrap();
|
||||
for (o, n) in [("/", 3), ("g", 3), ("has", 4), ("d", 3), ("e", 4)] {
|
||||
let attrs = match o {
|
||||
"/" => f.root().attrs().unwrap(),
|
||||
"d" | "e" => f.dataset(o).unwrap().attrs().unwrap(),
|
||||
_ => f.group(o).unwrap().attrs().unwrap(),
|
||||
};
|
||||
assert_eq!(attrs.len(), n + extra, "{o}");
|
||||
assert!(matches!(attrs.get("a1"), Some(AttrValue::F64(x)) if *x == 1.5));
|
||||
}
|
||||
if dump {
|
||||
// Every object's attributes: 3 set by the editor on each of
|
||||
// the five, 2 existing, `extra` from h5py on each.
|
||||
let out = Command::new("h5dump").args(["-A", p]).output().unwrap();
|
||||
assert!(out.status.success(), "h5dump -A:\n{}", text(&out));
|
||||
let s = String::from_utf8_lossy(&out.stdout);
|
||||
assert_eq!(
|
||||
s.matches("ATTRIBUTE \"").count(),
|
||||
5 * (3 + extra) + 2,
|
||||
"h5dump -A:\n{s}"
|
||||
);
|
||||
}
|
||||
};
|
||||
check(0);
|
||||
// libhdf5 adds more: to 7 or 8 (compact), then past its limit.
|
||||
for extra in [4usize, 10] {
|
||||
py(&format!(
|
||||
"import h5py\n\
|
||||
with h5py.File({p:?}, 'r+') as f:\n\
|
||||
\x20 for o in ['/', 'g', 'has', 'd', 'e']:\n\
|
||||
\x20 for i in range({extra}): f[o].attrs[f'h{{i}}'] = i\n"
|
||||
));
|
||||
check(extra);
|
||||
check_tools(&path, dump);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -47,6 +47,8 @@ const MSG_EXTERNAL: u16 = 0x07;
|
||||
const MSG_ATTR_INFO: u16 = 0x15;
|
||||
/// Message flag: the message is shared (stored elsewhere).
|
||||
const MSG_FLAG_SHARED: u8 = 0x02;
|
||||
/// Message flag: the message must not be shared (`H5O_MSG_FLAG_DONTSHARE`).
|
||||
const MSG_FLAG_DONTSHARE: u8 = 0x04;
|
||||
|
||||
/// An HDF5 file opened for in-place modification.
|
||||
///
|
||||
@@ -815,7 +817,24 @@ impl FileEditor {
|
||||
if let Some(i) = existing {
|
||||
hdr.delete(img, i)?;
|
||||
}
|
||||
// libhdf5 counts a version-2 header's attributes through its
|
||||
// Attribute Info message and reports none without one; like
|
||||
// H5O__attr_create, add it when missing: before the attribute
|
||||
// when free space holds both (libhdf5's order), else after it,
|
||||
// so that a new continuation chunk made for the attribute has
|
||||
// room for it too.
|
||||
let ainfo = (hdr.version == 2 && hdr.find(MSG_ATTR_INFO).is_none())
|
||||
.then(|| attr_info_message(hdr.flags, img.os));
|
||||
let ainfo_first = ainfo
|
||||
.as_ref()
|
||||
.is_some_and(|a| hdr.has_free(a.len() + hdr.hsize() + body.len()));
|
||||
if let Some(a) = ainfo.as_ref().filter(|_| ainfo_first) {
|
||||
hdr.insert(img, MSG_ATTR_INFO, MSG_FLAG_DONTSHARE, a, None)?;
|
||||
}
|
||||
hdr.insert(img, MSG_ATTRIBUTE, 0, &body, None)?;
|
||||
if let Some(a) = ainfo.as_ref().filter(|_| !ainfo_first) {
|
||||
hdr.insert(img, MSG_ATTR_INFO, MSG_FLAG_DONTSHARE, a, None)?;
|
||||
}
|
||||
hdr.finish(img)
|
||||
})
|
||||
}
|
||||
@@ -900,6 +919,26 @@ fn attr_name(d: &[u8]) -> Result<&[u8], Error> {
|
||||
Ok(name.split(|&b| b == 0).next().unwrap_or(name))
|
||||
}
|
||||
|
||||
/// A new Attribute Info message for a version-2 header with flags
|
||||
/// `hdr_flags`, as `H5O__attr_create` makes it: version 0, creation order
|
||||
/// tracked / indexed as the header's flags say, maximum creation index 0,
|
||||
/// and no dense storage (undefined fractal heap and B-tree addresses).
|
||||
fn attr_info_message(hdr_flags: u8, os: u8) -> Vec<u8> {
|
||||
let track = hdr_flags & 0x04 != 0;
|
||||
let index = hdr_flags & 0x08 != 0;
|
||||
let mut b = vec![0u8, u8::from(track) | (u8::from(index) << 1)];
|
||||
if track {
|
||||
b.extend_from_slice(&0u16.to_le_bytes());
|
||||
}
|
||||
let undef_addr = vec![0xffu8; os as usize];
|
||||
b.extend_from_slice(&undef_addr);
|
||||
b.extend_from_slice(&undef_addr);
|
||||
if index {
|
||||
b.extend_from_slice(&undef_addr);
|
||||
}
|
||||
b
|
||||
}
|
||||
|
||||
/// A version-2 header's limit on compact attributes: stored when its flags
|
||||
/// say so, else libhdf5's default of 8.
|
||||
fn max_compact_attrs(img: &Image<'_>, hdr: &Header) -> Result<u16, Error> {
|
||||
|
||||
@@ -326,6 +326,11 @@ impl Header {
|
||||
.map(|(i, _)| i)
|
||||
}
|
||||
|
||||
/// Whether free space in the header can take a body of `len` bytes.
|
||||
pub(crate) fn has_free(&self, len: usize) -> bool {
|
||||
self.best_nil(self.padded(len)).is_some()
|
||||
}
|
||||
|
||||
/// Put a message into slot `i` (a NIL message, or a message being
|
||||
/// moved away), splitting off the rest as a NIL message.
|
||||
fn place(
|
||||
|
||||
Reference in New Issue
Block a user