Merge feat/attr-fidelity: attrs() reports every attribute; unsigned arrays stay unsigned

Co-Authored-By: Claude Fable 5.1 <[email protected]>

CHANGELOG.md

@@ -41,6 +41,20 @@
   object_path }` (was `PathNotFound`), and a dataset whose raw data lives in
   external files (message 0x0007, now a known `MessageType`) is
   `ExternalDataFilesUnsupported` (it would otherwise read as fill values).
+- **`attrs()` no longer drops attributes.** Any attribute whose datatype had
+  no `AttrValue` variant was omitted with no error — including every Python
+  `bool` (h5py stores `attrs["flag"] = True` as an enum), complex numbers,
+  compound values and object references. Now:
+  - numpy/h5py-style booleans (an enum of exactly `FALSE`=0 / `TRUE`=1) decode
+    as `I64` / `I64Array` of 0/1;
+  - new `AttrValue::U64Array` keeps unsigned arrays unsigned (they were cast to
+    `I64Array`, so values above `i64::MAX` came back negative). **Behaviour
+    change:** code matching `I64Array` for an unsigned attribute must also
+    match `U64Array` (the netCDF-4 CF helpers and Python bindings do);
+  - new `AttrValue::Raw { datatype, shape, data }` carries everything else
+    verbatim, decodable with `clawhdf5_format::data_read` against `datatype`.
+  Both new variants are writable, so an attribute can be copied between files
+  unchanged. Python receives `Raw` as `{"dtype", "shape", "data"}`.
 - All of the above are covered by h5py interop tests under both default and
   `libver='latest'` bounds, compared against h5py's own readback.
 

crates/clawhdf5-format/src/type_builders.rs

@@ -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 ----

crates/clawhdf5-netcdf4/src/cf.rs

@@ -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));
+        }
         _ => {}
     }
 

crates/clawhdf5-py/src/lib.rs

@@ -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()
+        }
     }
 }
 

crates/clawhdf5/src/types.rs

@@ -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) {
+        // 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,
     }
 }

crates/clawhdf5/tests/h5py_interop_tests.rs

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

docs/known-issues.md

@@ -100,13 +100,19 @@ name padding, whereas v2 keeps the 8-byte name padding and has no array fields.
 
 ## Attributes with unsupported datatypes are silently dropped
 
-**Status:** open.
+**Status:** fixed 2026-09-19.
 
-**Summary:** `Dataset::attrs()` / `Group::attrs()` in the `clawhdf5` facade return
-only attributes convertible to `AttrValue`. An attribute with, e.g., a compound
-datatype is omitted from the map with no error or indication that it exists.
-Planned: surface these as an explicit `AttrValue` variant (raw bytes + datatype)
-or an error, as part of the "no silent skips" robustness work.
+**Summary:** `Dataset::attrs()` / `Group::attrs()` returned only attributes
+convertible to `AttrValue` and omitted the rest without any indication — every
+Python `bool` (an HDF5 enum), complex, compound and reference attributes. Unsigned
+64-bit arrays were also cast to `I64Array`, turning values above `i64::MAX`
+negative.
+
+**Fix:** booleans decode as 0/1 integers, `AttrValue::U64Array` keeps unsigned
+arrays unsigned, and `AttrValue::Raw { datatype, shape, data }` carries any other
+attribute verbatim. Both new variants are writable. Still lossy: a
+multi-dimensional numeric attribute is returned as a flat array (its shape is
+not reported).
 
 ## B-tree v2 chunk index (layout v4, index type 5) is not supported