From 04f153512edba15b8cc447da090647da6d357b99 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Beck=20K=C3=B6nig?= Date: Mon, 7 Sep 2026 20:19:44 +0200 Subject: [PATCH 1/2] Make AttrCreationOrder an enum The bitflags allowed INDEXED without TRACKED, a combination HDF5 rejects when the property list is built, so the invalid state was only caught at runtime. The enum with Untracked, Tracked and Indexed cannot express it, and matches LinkCreationOrder. --- CHANGELOG.md | 1 + hdf5/src/hl/plist/common.rs | 66 +++++++++++++++++++++-------- hdf5/src/hl/plist/dataset_create.rs | 13 +++--- hdf5/src/hl/plist/file_create.rs | 13 +++--- hdf5/tests/test_plist.rs | 18 ++++---- 5 files changed, 73 insertions(+), 38 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c1489c3d..115b7ee6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ - Changed `Group::iter_visit` and `Group::iter_visit_default` to take an `FnMut(&str, LinkInfo) -> Result<()>` closure instead of an accumulator and a `bool` closure, and `LinkInfo::is_utf8` to `LinkInfo::char_encoding` (breaking change). An error returned by the closure is propagated, and `Group::groups`, `Group::datasets`, `Group::committed_datatypes` and `Group::member_names` now fail instead of returning a truncated list when a link cannot be resolved - Added `Group::member_names_by` and `Group::links` to list the links along an index type in an order, `Group::find_link` to stop a link iteration with a value, and `Group::iter_visit_from` with `LinkCursor` to skip links and resume a stopped iteration - Added `FileCreateBuilder::link_creation_order`, `GroupCreateBuilder::link_creation_order`, the matching getters and `LinkCreationOrder`, so a group can track and index link creation order +- Changed `AttrCreationOrder` from bitflags to an enum with `Untracked`, `Tracked` and `Indexed`, matching `LinkCreationOrder` (breaking change) ## hdf5-derive unreleased ## hdf5-types unreleased ## hdf5-sys unreleased diff --git a/hdf5/src/hl/plist/common.rs b/hdf5/src/hl/plist/common.rs index 4f48b53b..dec1f219 100644 --- a/hdf5/src/hl/plist/common.rs +++ b/hdf5/src/hl/plist/common.rs @@ -2,8 +2,6 @@ use std::os::raw::c_uint; use hdf5_sys::h5p::{H5P_CRT_ORDER_INDEXED, H5P_CRT_ORDER_TRACKED}; -use bitflags::bitflags; - /// Attribute storage phase change thresholds. /// /// These thresholds determine the point at which attribute storage changes from @@ -30,21 +28,55 @@ impl Default for AttrPhaseChange { } } -bitflags! { - /// Flags for tracking and indexing attribute creation order of an object. - /// - /// Default behavior is that attribute creation order is neither tracked nor indexed. - /// - /// Note that if a creation order index is to be built, it must be specified in - /// the object creation property list. HDF5 currently provides no mechanism to turn - /// on attribute creation order tracking at object creation time and to build the - /// index later. - #[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] - pub struct AttrCreationOrder: u32 { - /// Attribute creation order is tracked but not necessarily indexed. - const TRACKED = H5P_CRT_ORDER_TRACKED as _; - /// Attribute creation order is indexed (requires to be tracked). - const INDEXED = H5P_CRT_ORDER_INDEXED as _; +/// Tracking of attribute creation order on an object. +/// +/// By default attribute creation order is not recorded. `Tracked` records the order +/// in which attributes are created. `Indexed` also maintains an index for iterating +/// attributes by creation order. +/// +/// The setting is fixed in the creation property list. HDF5 provides no way to +/// turn on tracking or build the index after the object exists. +/// +/// # Examples +/// +/// ``` +/// use hdf5_metno::plist::FileCreateBuilder; +/// use hdf5_metno::plist::file_create::AttrCreationOrder; +/// +/// let fcpl = FileCreateBuilder::new().attr_creation_order(AttrCreationOrder::Indexed).finish()?; +/// assert_eq!(fcpl.attr_creation_order(), AttrCreationOrder::Indexed); +/// # Ok::<(), hdf5_metno::Error>(()) +/// ``` +#[derive(Clone, Copy, Debug, PartialEq, Eq, Default)] +pub enum AttrCreationOrder { + /// Attribute creation order is not recorded. + #[default] + Untracked, + /// Attribute creation order is recorded. + Tracked, + /// Attribute creation order is recorded and indexed. + Indexed, +} + +impl AttrCreationOrder { + pub(crate) fn from_flags(flags: c_uint) -> Self { + if flags & H5P_CRT_ORDER_INDEXED != 0 { + Self::Indexed + } else if flags & H5P_CRT_ORDER_TRACKED != 0 { + Self::Tracked + } else { + Self::Untracked + } + } +} + +impl From for c_uint { + fn from(v: AttrCreationOrder) -> Self { + match v { + AttrCreationOrder::Untracked => 0, + AttrCreationOrder::Tracked => H5P_CRT_ORDER_TRACKED, + AttrCreationOrder::Indexed => H5P_CRT_ORDER_TRACKED | H5P_CRT_ORDER_INDEXED, + } } } diff --git a/hdf5/src/hl/plist/dataset_create.rs b/hdf5/src/hl/plist/dataset_create.rs index 0bd21df4..f185a5c0 100644 --- a/hdf5/src/hl/plist/dataset_create.rs +++ b/hdf5/src/hl/plist/dataset_create.rs @@ -664,7 +664,9 @@ impl DatasetCreateBuilder { self } - /// Sets whether to track and/or index the dataset's attribute creation order. + /// Sets whether the dataset's attribute creation order is tracked and indexed. + /// + /// See [`AttrCreationOrder`] for the available settings. pub fn attr_creation_order(&mut self, attr_creation_order: AttrCreationOrder) -> &mut Self { self.attr_creation_order = Some(attr_creation_order); self @@ -726,7 +728,7 @@ impl DatasetCreateBuilder { h5try!(H5Pset_attr_phase_change(id, v.max_compact as _, v.min_dense as _)); } if let Some(v) = self.attr_creation_order { - h5try!(H5Pset_attr_creation_order(id, v.bits() as _)); + h5try!(H5Pset_attr_creation_order(id, v.into())); } Ok(()) } @@ -1003,11 +1005,12 @@ impl DatasetCreate { #[doc(hidden)] pub fn get_attr_creation_order(&self) -> Result { - h5get!(H5Pget_attr_creation_order(self.id()): c_uint) - .map(AttrCreationOrder::from_bits_truncate) + h5get!(H5Pget_attr_creation_order(self.id()): c_uint).map(AttrCreationOrder::from_flags) } - /// Returns flags for whether attribute creation order will be tracked/indexed. + /// Returns whether the dataset's attribute creation order is tracked and indexed. + /// + /// Returns [`AttrCreationOrder::Untracked`] if the property cannot be read. pub fn attr_creation_order(&self) -> AttrCreationOrder { self.get_attr_creation_order().unwrap_or_default() } diff --git a/hdf5/src/hl/plist/file_create.rs b/hdf5/src/hl/plist/file_create.rs index 972e2263..fbbf2c22 100644 --- a/hdf5/src/hl/plist/file_create.rs +++ b/hdf5/src/hl/plist/file_create.rs @@ -405,9 +405,9 @@ impl FileCreateBuilder { self } - /// Sets flags for tracking and indexing attribute creation order. + /// Sets whether attribute creation order is tracked and indexed. /// - /// For further details, see [`AttrCreationOrder`](struct.AttrCreationOrder.html). + /// See [`AttrCreationOrder`] for the available settings. pub fn attr_creation_order(&mut self, attr_creation_order: AttrCreationOrder) -> &mut Self { self.attr_creation_order = Some(attr_creation_order); self @@ -481,7 +481,7 @@ impl FileCreateBuilder { h5try!(H5Pset_attr_phase_change(id, v.max_compact as _, v.min_dense as _)); } if let Some(v) = self.attr_creation_order { - h5try!(H5Pset_attr_creation_order(id, v.bits() as _)); + h5try!(H5Pset_attr_creation_order(id, v.into())); } if let Some(v) = self.link_creation_order { h5try!(H5Pset_link_creation_order(id, v.into())); @@ -681,11 +681,12 @@ impl FileCreate { #[doc(hidden)] pub fn get_attr_creation_order(&self) -> Result { - h5get!(H5Pget_attr_creation_order(self.id()): c_uint) - .map(AttrCreationOrder::from_bits_truncate) + h5get!(H5Pget_attr_creation_order(self.id()): c_uint).map(AttrCreationOrder::from_flags) } - /// Returns flags for tracking and indexing attribute creation order. + /// Returns whether attribute creation order is tracked and indexed. + /// + /// Returns [`AttrCreationOrder::Untracked`] if the property cannot be read. pub fn attr_creation_order(&self) -> AttrCreationOrder { self.get_attr_creation_order().unwrap_or_default() } diff --git a/hdf5/tests/test_plist.rs b/hdf5/tests/test_plist.rs index ea05cde8..e11145ec 100644 --- a/hdf5/tests/test_plist.rs +++ b/hdf5/tests/test_plist.rs @@ -158,11 +158,10 @@ fn test_fcpl_attr_phase_change() -> hdf5::Result<()> { #[test] fn test_fcpl_attr_creation_order() -> hdf5::Result<()> { - assert_eq!(FC::try_new()?.get_attr_creation_order()?.bits(), 0); - assert_eq!(FC::try_new()?.attr_creation_order().bits(), 0); - test_pl!(FC, attr_creation_order: AttrCreationOrder::TRACKED); - test_pl!(FC, attr_creation_order: AttrCreationOrder::TRACKED | AttrCreationOrder::INDEXED); - assert!(FCB::new().attr_creation_order(AttrCreationOrder::INDEXED).finish().is_err()); + assert_eq!(FC::try_new()?.get_attr_creation_order()?, AttrCreationOrder::Untracked); + assert_eq!(FC::try_new()?.attr_creation_order(), AttrCreationOrder::Untracked); + test_pl!(FC, attr_creation_order: AttrCreationOrder::Tracked); + test_pl!(FC, attr_creation_order: AttrCreationOrder::Indexed); Ok(()) } @@ -922,11 +921,10 @@ fn test_dcpl_attr_phase_change() -> hdf5::Result<()> { #[test] fn test_dcpl_attr_creation_order() -> hdf5::Result<()> { - assert_eq!(DC::try_new()?.get_attr_creation_order()?.bits(), 0); - assert_eq!(DC::try_new()?.attr_creation_order().bits(), 0); - test_pl!(DC, attr_creation_order: AttrCreationOrder::TRACKED); - test_pl!(DC, attr_creation_order: AttrCreationOrder::TRACKED | AttrCreationOrder::INDEXED); - assert!(DCB::new().attr_creation_order(AttrCreationOrder::INDEXED).finish().is_err()); + assert_eq!(DC::try_new()?.get_attr_creation_order()?, AttrCreationOrder::Untracked); + assert_eq!(DC::try_new()?.attr_creation_order(), AttrCreationOrder::Untracked); + test_pl!(DC, attr_creation_order: AttrCreationOrder::Tracked); + test_pl!(DC, attr_creation_order: AttrCreationOrder::Indexed); Ok(()) } From 97f6bc22af7d84246ab9e4fa45bde68263b0baa3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc=20Beck=20K=C3=B6nig?= Date: Mon, 7 Sep 2026 20:21:23 +0200 Subject: [PATCH 2/2] Add attr_creation_order and attr_phase_change to GroupCreate FileCreate and DatasetCreate already expose both. A group tracking attribute creation order is what h5py's track_order=True and netCDF-4 produce, and it is required for iterating attributes by creation order. --- CHANGELOG.md | 1 + hdf5/src/hl/group.rs | 27 +++++++++++++- hdf5/src/hl/plist/group_create.rs | 61 +++++++++++++++++++++++++++++-- hdf5/tests/test_plist.rs | 31 ++++++++++++++++ 4 files changed, 116 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 115b7ee6..05870896 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Added `Group::member_names_by` and `Group::links` to list the links along an index type in an order, `Group::find_link` to stop a link iteration with a value, and `Group::iter_visit_from` with `LinkCursor` to skip links and resume a stopped iteration - Added `FileCreateBuilder::link_creation_order`, `GroupCreateBuilder::link_creation_order`, the matching getters and `LinkCreationOrder`, so a group can track and index link creation order - Changed `AttrCreationOrder` from bitflags to an enum with `Untracked`, `Tracked` and `Indexed`, matching `LinkCreationOrder` (breaking change) +- Added `GroupCreateBuilder::attr_creation_order` and `GroupCreateBuilder::attr_phase_change` with the matching `GroupCreate` getters ## hdf5-derive unreleased ## hdf5-types unreleased ## hdf5-sys unreleased diff --git a/hdf5/src/hl/group.rs b/hdf5/src/hl/group.rs index 546ce291..0f48707f 100644 --- a/hdf5/src/hl/group.rs +++ b/hdf5/src/hl/group.rs @@ -864,7 +864,7 @@ impl Group { #[cfg(test)] pub mod tests { - use crate::hl::plist::common::LinkCreationOrder; + use crate::hl::plist::common::{AttrCreationOrder, AttrPhaseChange, LinkCreationOrder}; use crate::hl::plist::file_access::FileCloseDegree; #[cfg(feature = "1.10.2")] use crate::hl::plist::file_access::LibraryVersion; @@ -1073,6 +1073,31 @@ pub mod tests { }) } + #[test] + pub fn test_group_attr_creation_order() { + with_tmp_file(|file| { + let group = file + .create_group_builder() + .with_gcpl(|gcpl| { + gcpl.attr_creation_order(AttrCreationOrder::Indexed).attr_phase_change(2, 1) + }) + .create("g") + .unwrap(); + for name in ["c", "a", "b"] { + group.new_attr::().create(name).unwrap(); + } + + let gcpl = group.gcpl().unwrap(); + assert_eq!(gcpl.attr_creation_order(), AttrCreationOrder::Indexed); + assert_eq!(gcpl.attr_phase_change(), AttrPhaseChange { max_compact: 2, min_dense: 1 }); + assert_eq!(group.attr_names().unwrap(), ["a", "b", "c"]); + + let gcpl = file.create_group("untracked").unwrap().gcpl().unwrap(); + assert_eq!(gcpl.attr_creation_order(), AttrCreationOrder::Untracked); + assert_eq!(gcpl.attr_phase_change(), AttrPhaseChange::default()); + }) + } + // `obj_track_times` maps to a flag bit in the object header. Only a version-2 // object header carries that flag (`H5O_HDR_STORE_TIMES` in the header prefix), // so only a v2 header can record the setting and report it back through diff --git a/hdf5/src/hl/plist/group_create.rs b/hdf5/src/hl/plist/group_create.rs index f1083f65..19e36e20 100644 --- a/hdf5/src/hl/plist/group_create.rs +++ b/hdf5/src/hl/plist/group_create.rs @@ -4,12 +4,13 @@ use std::fmt::{self, Debug}; use std::ops::Deref; use hdf5_sys::h5p::{ - H5Pcreate, H5Pget_link_creation_order, H5Pget_obj_track_times, H5Pset_link_creation_order, - H5Pset_obj_track_times, + H5Pcreate, H5Pget_attr_creation_order, H5Pget_attr_phase_change, H5Pget_link_creation_order, + H5Pget_obj_track_times, H5Pset_attr_creation_order, H5Pset_attr_phase_change, + H5Pset_link_creation_order, H5Pset_obj_track_times, }; use crate::globals::H5P_GROUP_CREATE; -pub use crate::hl::plist::common::LinkCreationOrder; +pub use crate::hl::plist::common::{AttrCreationOrder, AttrPhaseChange, LinkCreationOrder}; use crate::internal_prelude::*; /// Group creation properties. @@ -49,6 +50,8 @@ impl Debug for GroupCreate { let mut formatter = f.debug_struct("GroupCreate"); formatter.field("obj_track_times", &self.obj_track_times()); formatter.field("link_creation_order", &self.link_creation_order()); + formatter.field("attr_phase_change", &self.attr_phase_change()); + formatter.field("attr_creation_order", &self.attr_creation_order()); formatter.finish() } } @@ -74,6 +77,8 @@ impl Eq for GroupCreate {} pub struct GroupCreateBuilder { obj_track_times: Option, link_creation_order: Option, + attr_phase_change: Option, + attr_creation_order: Option, } impl GroupCreateBuilder { @@ -87,6 +92,9 @@ impl GroupCreateBuilder { let mut builder = Self::default(); builder.obj_track_times(plist.get_obj_track_times()?); builder.link_creation_order(plist.get_link_creation_order()?); + let apc = plist.get_attr_phase_change()?; + builder.attr_phase_change(apc.max_compact, apc.min_dense); + builder.attr_creation_order(plist.get_attr_creation_order()?); Ok(builder) } @@ -106,6 +114,22 @@ impl GroupCreateBuilder { self } + /// Sets the group's attribute storage phase change thresholds. + /// + /// See [`AttrPhaseChange`] for the meaning of the thresholds. + pub fn attr_phase_change(&mut self, max_compact: u32, min_dense: u32) -> &mut Self { + self.attr_phase_change = Some(AttrPhaseChange { max_compact, min_dense }); + self + } + + /// Sets whether the group's attribute creation order is tracked and indexed. + /// + /// See [`AttrCreationOrder`] for the available settings. + pub fn attr_creation_order(&mut self, attr_creation_order: AttrCreationOrder) -> &mut Self { + self.attr_creation_order = Some(attr_creation_order); + self + } + fn populate_plist(&self, id: hid_t) -> Result<()> { if let Some(v) = self.obj_track_times { h5try!(H5Pset_obj_track_times(id, hbool_t::from(v))); @@ -113,6 +137,12 @@ impl GroupCreateBuilder { if let Some(v) = self.link_creation_order { h5try!(H5Pset_link_creation_order(id, v.into())); } + if let Some(v) = self.attr_phase_change { + h5try!(H5Pset_attr_phase_change(id, v.max_compact as _, v.min_dense as _)); + } + if let Some(v) = self.attr_creation_order { + h5try!(H5Pset_attr_creation_order(id, v.into())); + } Ok(()) } @@ -168,4 +198,29 @@ impl GroupCreate { pub fn link_creation_order(&self) -> LinkCreationOrder { self.get_link_creation_order().unwrap_or_default() } + + #[doc(hidden)] + pub fn get_attr_phase_change(&self) -> Result { + h5get!(H5Pget_attr_phase_change(self.id()): c_uint, c_uint) + .map(|(mc, md)| AttrPhaseChange { max_compact: mc as _, min_dense: md as _ }) + } + + /// Returns the group's attribute storage phase change thresholds. + /// + /// Returns the HDF5 defaults if the property cannot be read. + pub fn attr_phase_change(&self) -> AttrPhaseChange { + self.get_attr_phase_change().unwrap_or_default() + } + + #[doc(hidden)] + pub fn get_attr_creation_order(&self) -> Result { + h5get!(H5Pget_attr_creation_order(self.id()): c_uint).map(AttrCreationOrder::from_flags) + } + + /// Returns whether the group's attribute creation order is tracked and indexed. + /// + /// Returns [`AttrCreationOrder::Untracked`] if the property cannot be read. + pub fn attr_creation_order(&self) -> AttrCreationOrder { + self.get_attr_creation_order().unwrap_or_default() + } } diff --git a/hdf5/tests/test_plist.rs b/hdf5/tests/test_plist.rs index e11145ec..63e9de1a 100644 --- a/hdf5/tests/test_plist.rs +++ b/hdf5/tests/test_plist.rs @@ -4,6 +4,7 @@ use std::str::FromStr; use hdf5::dataset::*; use hdf5::file::*; use hdf5::plist::*; +use hdf5::{MajorErrorCode, MinorErrorCode}; use hdf5_metno as hdf5; macro_rules! test_pl { @@ -663,6 +664,36 @@ fn test_gcpl_obj_track_times() -> hdf5::Result<()> { Ok(()) } +#[test] +fn test_gcpl_attr_phase_change() -> hdf5::Result<()> { + assert_eq!(GC::try_new()?.get_attr_phase_change()?, AttrPhaseChange::default()); + assert_eq!(GC::try_new()?.attr_phase_change(), AttrPhaseChange::default()); + let pl = GCB::new().attr_phase_change(34, 21).finish()?; + let expected = AttrPhaseChange { max_compact: 34, min_dense: 21 }; + assert_eq!(pl.get_attr_phase_change()?, expected); + assert_eq!(pl.attr_phase_change(), expected); + assert_eq!(GCB::from_plist(&pl)?.finish()?.get_attr_phase_change()?, expected); + let err = GCB::new().attr_phase_change(12, 34).finish().unwrap_err(); + assert!(err.contains_major(MajorErrorCode::Args), "{err:?}"); + assert!(err.contains_minor(MinorErrorCode::BadRange), "{err:?}"); + Ok(()) +} + +#[test] +fn test_gcpl_attr_creation_order() -> hdf5::Result<()> { + assert_eq!(GC::try_new()?.get_attr_creation_order()?, AttrCreationOrder::Untracked); + assert_eq!(GC::try_new()?.attr_creation_order(), AttrCreationOrder::Untracked); + test_pl!(GC, attr_creation_order: AttrCreationOrder::Tracked); + test_pl!(GC, attr_creation_order: AttrCreationOrder::Indexed); + assert_eq!( + GCB::from_plist(&GCB::new().attr_creation_order(AttrCreationOrder::Indexed).finish()?)? + .finish()? + .attr_creation_order(), + AttrCreationOrder::Indexed + ); + Ok(()) +} + #[test] fn test_gcpl_link_creation_order() -> hdf5::Result<()> { assert_eq!(GC::try_new()?.get_link_creation_order()?, LinkCreationOrder::Untracked);