feat: attrs() reports every attribute; unsigned arrays stay unsigned
attrs() silently omitted any attribute whose datatype had no AttrValue variant
— including every Python bool, which h5py stores as an enum — plus complex,
compound and reference attributes, and cast unsigned 64-bit arrays to
I64Array so values above i64::MAX came back negative.
- numpy/h5py-style booleans (an enum of exactly FALSE=0 / TRUE=1 over an
integer base) decode as I64 / I64Array of 0/1.
- AttrValue::U64Array keeps unsigned arrays unsigned. Behaviour change: an
unsigned array attribute no longer arrives as I64Array; the netCDF-4 CF
helpers (_FillValue, valid_range) and the Python bindings handle it.
- AttrValue::Raw { datatype, shape, data } carries any other attribute
verbatim (also used when a value fails to decode as its declared type), so
the attribute list is always complete. Decodable with data_read against the
datatype; Python receives {"dtype", "shape", "data"}.
- Both new variants are writable, so attributes round-trip between files.
h5py interop tests cover reading 13 attribute kinds and h5py reading back a
compound and a u64 attribute written by clawhdf5.
Co-Authored-By: Claude Fable 5.1 <[email protected]>
This commit is contained in:
osobh
co-authored by
Claude Fable 5.1
Claude Fable 5.1
parent
5dd95a6cf8
commit
97ab658c11
@@ -279,6 +279,43 @@ pub(crate) fn build_attr_message(name: &str, value: &AttrValue) -> AttributeMess
|
||||
dataspace: scalar_ds(),
|
||||
raw_data: v.to_le_bytes().to_vec(),
|
||||
},
|
||||
AttrValue::U64Array(arr) => {
|
||||
let mut raw = Vec::with_capacity(arr.len() * 8);
|
||||
for v in arr {
|
||||
raw.extend_from_slice(&v.to_le_bytes());
|
||||
}
|
||||
AttributeMessage {
|
||||
name: name.to_string(),
|
||||
datatype: Datatype::FixedPoint {
|
||||
size: 8,
|
||||
byte_order: DatatypeByteOrder::LittleEndian,
|
||||
signed: false,
|
||||
bit_offset: 0,
|
||||
bit_precision: 64,
|
||||
},
|
||||
dataspace: simple_1d(arr.len() as u64),
|
||||
raw_data: raw,
|
||||
}
|
||||
}
|
||||
AttrValue::Raw {
|
||||
datatype,
|
||||
shape,
|
||||
data,
|
||||
} => AttributeMessage {
|
||||
name: name.to_string(),
|
||||
datatype: datatype.clone(),
|
||||
dataspace: if shape.is_empty() {
|
||||
scalar_ds()
|
||||
} else {
|
||||
Dataspace {
|
||||
space_type: DataspaceType::Simple,
|
||||
rank: shape.len() as u8,
|
||||
dimensions: shape.clone(),
|
||||
max_dimensions: None,
|
||||
}
|
||||
},
|
||||
raw_data: data.clone(),
|
||||
},
|
||||
AttrValue::String(s) => {
|
||||
let bytes = s.as_bytes();
|
||||
AttributeMessage {
|
||||
@@ -334,7 +371,7 @@ pub(crate) fn simple_1d(n: u64) -> Dataspace {
|
||||
|
||||
// ---- Attribute values ----
|
||||
|
||||
/// Convenient attribute values for the write API.
|
||||
/// Attribute values, for both the write API and what reading returns.
|
||||
#[derive(Debug, Clone)]
|
||||
pub enum AttrValue {
|
||||
F64(f64),
|
||||
@@ -342,8 +379,21 @@ pub enum AttrValue {
|
||||
I64(i64),
|
||||
I64Array(Vec<i64>),
|
||||
U64(u64),
|
||||
/// Unsigned integers, kept unsigned so values above `i64::MAX` survive.
|
||||
U64Array(Vec<u64>),
|
||||
String(String),
|
||||
StringArray(Vec<String>),
|
||||
/// An attribute whose datatype has no dedicated variant above (compound,
|
||||
/// general enum, complex, reference, opaque, array, ...), carried verbatim
|
||||
/// so it is never silently lost: the datatype, the dataspace dimensions
|
||||
/// (empty for a scalar) and the element bytes exactly as stored. Decode
|
||||
/// `data` with `clawhdf5_format::data_read` (e.g. `read_compound_fields`)
|
||||
/// against `datatype`. Writing a `Raw` value stores it back unchanged.
|
||||
Raw {
|
||||
datatype: Datatype,
|
||||
shape: Vec<u64>,
|
||||
data: Vec<u8>,
|
||||
},
|
||||
}
|
||||
|
||||
// ---- Dataset builder ----
|
||||
|
||||
@@ -142,6 +142,7 @@ fn get_fill_value(attrs: &HashMap<String, AttrValue>, key: &str) -> Option<FillV
|
||||
Some(AttrValue::String(s)) => Some(FillValue::String(s.clone())),
|
||||
Some(AttrValue::F64Array(arr)) if !arr.is_empty() => Some(FillValue::Float(arr[0])),
|
||||
Some(AttrValue::I64Array(arr)) if !arr.is_empty() => Some(FillValue::Int(arr[0])),
|
||||
Some(AttrValue::U64Array(arr)) if !arr.is_empty() => Some(FillValue::UInt(arr[0])),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -155,6 +156,10 @@ fn get_valid_range(attrs: &HashMap<String, AttrValue>) -> Option<(f64, f64)> {
|
||||
Some(AttrValue::I64Array(arr)) if arr.len() >= 2 => {
|
||||
return Some((arr[0] as f64, arr[1] as f64));
|
||||
}
|
||||
// Unsigned variables (NC_UBYTE..NC_UINT64) carry unsigned attributes.
|
||||
Some(AttrValue::U64Array(arr)) if arr.len() >= 2 => {
|
||||
return Some((arr[0] as f64, arr[1] as f64));
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
|
||||
@@ -128,10 +128,28 @@ pub(crate) fn attr_value_to_py(py: Python<'_>, val: &clawhdf5_rs::AttrValue) ->
|
||||
let list = pyo3::types::PyList::new(py, a).unwrap();
|
||||
list.into_any().unbind()
|
||||
}
|
||||
clawhdf5_rs::AttrValue::U64Array(a) => {
|
||||
let list = pyo3::types::PyList::new(py, a).unwrap();
|
||||
list.into_any().unbind()
|
||||
}
|
||||
clawhdf5_rs::AttrValue::StringArray(a) => {
|
||||
let list = pyo3::types::PyList::new(py, a).unwrap();
|
||||
list.into_any().unbind()
|
||||
}
|
||||
// No Python-side decoding for this datatype: hand back everything
|
||||
// needed to interpret it rather than dropping the attribute.
|
||||
clawhdf5_rs::AttrValue::Raw {
|
||||
datatype,
|
||||
shape,
|
||||
data,
|
||||
} => {
|
||||
let dict = pyo3::types::PyDict::new(py);
|
||||
dict.set_item("dtype", format!("{datatype:?}")).unwrap();
|
||||
dict.set_item("shape", shape).unwrap();
|
||||
dict.set_item("data", pyo3::types::PyBytes::new(py, data))
|
||||
.unwrap();
|
||||
dict.into_any().unbind()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
||||
@@ -163,13 +163,53 @@ pub(crate) fn attrs_to_map(
|
||||
) -> HashMap<String, AttrValue> {
|
||||
let mut map = HashMap::new();
|
||||
for attr in attrs {
|
||||
if let Some(val) = decode_attr_value(attr, file_data, offset_size, length_size) {
|
||||
map.insert(attr.name.clone(), val);
|
||||
}
|
||||
// Every attribute is reported. One that has no dedicated `AttrValue`
|
||||
// variant, or that fails to decode as its declared type, is returned
|
||||
// verbatim as `AttrValue::Raw` rather than dropped — a partial
|
||||
// attribute list with no indication anything is missing is worse than
|
||||
// an undecoded value.
|
||||
let val =
|
||||
decode_attr_value(attr, file_data, offset_size, length_size).unwrap_or_else(|| {
|
||||
AttrValue::Raw {
|
||||
datatype: attr.datatype.clone(),
|
||||
shape: attr.dataspace.dimensions.clone(),
|
||||
data: attr.raw_data.clone(),
|
||||
}
|
||||
});
|
||||
map.insert(attr.name.clone(), val);
|
||||
}
|
||||
map
|
||||
}
|
||||
|
||||
/// `Some(values)` if `attr` is a numpy/h5py-style boolean: an enumeration over
|
||||
/// an integer base type whose members are exactly `FALSE` = 0 and `TRUE` = 1.
|
||||
/// This is how `attrs["flag"] = True` is stored. Reported as 0/1 integers.
|
||||
fn decode_bool_enum(attr: &clawhdf5_format::attribute::AttributeMessage) -> Option<Vec<i64>> {
|
||||
use clawhdf5_format::datatype::Datatype;
|
||||
|
||||
let Datatype::Enumeration {
|
||||
base_type, members, ..
|
||||
} = &attr.datatype
|
||||
else {
|
||||
return None;
|
||||
};
|
||||
if members.len() != 2 {
|
||||
return None;
|
||||
}
|
||||
let member_value = |name: &str| {
|
||||
let m = members.iter().find(|m| m.name.eq_ignore_ascii_case(name))?;
|
||||
clawhdf5_format::data_read::read_as_i64(&m.value, base_type)
|
||||
.ok()?
|
||||
.first()
|
||||
.copied()
|
||||
};
|
||||
if member_value("FALSE")? != 0 || member_value("TRUE")? != 1 {
|
||||
return None;
|
||||
}
|
||||
let values = clawhdf5_format::data_read::read_as_i64(&attr.raw_data, base_type).ok()?;
|
||||
values.iter().all(|v| *v == 0 || *v == 1).then_some(values)
|
||||
}
|
||||
|
||||
fn decode_attr_value(
|
||||
attr: &clawhdf5_format::attribute::AttributeMessage,
|
||||
file_data: &[u8],
|
||||
@@ -200,12 +240,7 @@ fn decode_attr_value(
|
||||
if vals.len() == 1 {
|
||||
Some(AttrValue::U64(vals[0]))
|
||||
} else {
|
||||
// No U64Array variant, store as I64Array.
|
||||
// NOTE: This cast is lossy for values > i64::MAX (bit 63 set).
|
||||
// Those values will appear as negative i64. A dedicated U64Array
|
||||
// variant would be needed to handle the full u64 range.
|
||||
let i64_vals: Vec<i64> = vals.iter().map(|&v| v as i64).collect();
|
||||
Some(AttrValue::I64Array(i64_vals))
|
||||
Some(AttrValue::U64Array(vals))
|
||||
}
|
||||
}
|
||||
Datatype::String { .. } => {
|
||||
@@ -228,6 +263,14 @@ fn decode_attr_value(
|
||||
Some(AttrValue::StringArray(strings))
|
||||
}
|
||||
}
|
||||
Datatype::Enumeration { .. } => {
|
||||
let vals = decode_bool_enum(attr)?;
|
||||
if vals.len() == 1 {
|
||||
Some(AttrValue::I64(vals[0]))
|
||||
} else {
|
||||
Some(AttrValue::I64Array(vals))
|
||||
}
|
||||
}
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
|
||||
@@ -784,3 +784,132 @@ with h5py.File("links_{tag}.h5", "w"{kwargs}) as f:
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Attribute fidelity: nothing is dropped, unsigned stays unsigned
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
/// `attrs()` used to omit, without any error, every attribute whose datatype
|
||||
/// had no `AttrValue` variant — including every Python `bool` (stored as an
|
||||
/// enum) — and to wrap unsigned 64-bit arrays into negative `i64`s.
|
||||
#[test]
|
||||
fn h5py_attribute_kinds_are_all_reported() {
|
||||
skip_if_no_python!();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("attrs_kinds.h5");
|
||||
let path_str = path.display().to_string();
|
||||
let script = format!(
|
||||
r#"
|
||||
import h5py, numpy as np
|
||||
with h5py.File("{path_str}", "w") as f:
|
||||
d = f.create_dataset("d", data=np.arange(3))
|
||||
a = d.attrs
|
||||
a["float"] = 1.5; a["int"] = 7; a["str"] = "hello"; a["int_array"] = np.arange(4)
|
||||
a["str_list"] = ["a", "bc"]
|
||||
a["uint64_array"] = np.array([2**63, 1], dtype="u8")
|
||||
a["bool_true"] = True
|
||||
a["bool_false"] = False
|
||||
a["bool_array"] = np.array([True, False, True])
|
||||
a["complex"] = 1 + 2j
|
||||
a["compound"] = np.array([(1, 2.5)], dtype=[("a", "<i4"), ("b", "<f8")])
|
||||
a["object_ref"] = d.ref
|
||||
a["color"] = np.array(2, dtype=h5py.enum_dtype({{"RED": 0, "GREEN": 1, "BLUE": 2}}, basetype="i1"))
|
||||
print(len(a))
|
||||
"#
|
||||
);
|
||||
let written: usize = run_python_output(&script).trim().parse().unwrap();
|
||||
|
||||
let file = File::open(&path).unwrap();
|
||||
let attrs = file.dataset("d").unwrap().attrs().unwrap();
|
||||
assert_eq!(
|
||||
attrs.len(),
|
||||
written,
|
||||
"every attribute is reported: {attrs:?}"
|
||||
);
|
||||
|
||||
// Booleans decode as 0/1.
|
||||
assert!(matches!(attrs["bool_true"], AttrValue::I64(1)));
|
||||
assert!(matches!(attrs["bool_false"], AttrValue::I64(0)));
|
||||
assert!(matches!(&attrs["bool_array"], AttrValue::I64Array(v) if v == &[1, 0, 1]));
|
||||
// Unsigned stays unsigned.
|
||||
assert!(matches!(&attrs["uint64_array"], AttrValue::U64Array(v) if v == &[1u64 << 63, 1]));
|
||||
|
||||
// A general enum is not a boolean: kept verbatim, member names intact.
|
||||
match &attrs["color"] {
|
||||
AttrValue::Raw { datatype, data, .. } => {
|
||||
assert_eq!(data, &[2]);
|
||||
assert!(format!("{datatype:?}").contains("BLUE"));
|
||||
}
|
||||
other => panic!("color: {other:?}"),
|
||||
}
|
||||
// A compound attribute is kept verbatim and decodes against its datatype.
|
||||
match &attrs["compound"] {
|
||||
AttrValue::Raw {
|
||||
datatype,
|
||||
shape,
|
||||
data,
|
||||
} => {
|
||||
assert_eq!(shape, &[1]);
|
||||
let fields = clawhdf5_format::data_read::read_compound_fields(data, datatype).unwrap();
|
||||
assert_eq!(fields[0].name, "a");
|
||||
let b =
|
||||
clawhdf5_format::data_read::read_as_f64(&fields[1].raw_data, &fields[1].datatype)
|
||||
.unwrap();
|
||||
assert_eq!(b, vec![2.5]);
|
||||
}
|
||||
other => panic!("compound: {other:?}"),
|
||||
}
|
||||
for name in ["complex", "object_ref"] {
|
||||
assert!(
|
||||
matches!(attrs[name], AttrValue::Raw { .. }),
|
||||
"{name}: {:?}",
|
||||
attrs[name]
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
/// `Raw` and `U64Array` are writable, so an attribute read from one file can
|
||||
/// be stored in another unchanged.
|
||||
#[test]
|
||||
fn clawhdf5_writes_raw_and_unsigned_attrs_h5py_reads() {
|
||||
skip_if_no_python!();
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let src = dir.path().join("src.h5");
|
||||
let dst = dir.path().join("dst.h5");
|
||||
let (src_str, dst_str) = (src.display().to_string(), dst.display().to_string());
|
||||
run_python(&format!(
|
||||
r#"
|
||||
import h5py, numpy as np
|
||||
with h5py.File("{src_str}", "w") as f:
|
||||
d = f.create_dataset("d", data=np.arange(3))
|
||||
d.attrs["compound"] = np.array([(1, 2.5), (3, 4.5)], dtype=[("a", "<i4"), ("b", "<f8")])
|
||||
"#
|
||||
));
|
||||
let compound = File::open(&src)
|
||||
.unwrap()
|
||||
.dataset("d")
|
||||
.unwrap()
|
||||
.attrs()
|
||||
.unwrap()["compound"]
|
||||
.clone();
|
||||
|
||||
let mut builder = FileBuilder::new();
|
||||
let ds = builder.create_dataset("d");
|
||||
ds.with_f64_data(&[1.0]);
|
||||
ds.set_attr("compound", compound);
|
||||
ds.set_attr("big", AttrValue::U64Array(vec![u64::MAX, 0, 1 << 63]));
|
||||
builder.write(&dst).unwrap();
|
||||
|
||||
let out = run_python_output(&format!(
|
||||
r#"
|
||||
import h5py
|
||||
with h5py.File("{dst_str}", "r") as f:
|
||||
a = f["d"].attrs
|
||||
print(a["compound"].tolist(), a["compound"].dtype.names, a["big"].tolist(), a["big"].dtype)
|
||||
"#
|
||||
));
|
||||
assert_eq!(
|
||||
out.trim(),
|
||||
"[(1, 2.5), (3, 4.5)] ('a', 'b') [18446744073709551615, 0, 9223372036854775808] uint64"
|
||||
);
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user