From 92fb0830e06e18383c91ff674cee5eeff2ae6917 Mon Sep 17 00:00:00 2001 From: osobh Date: Sun, 27 Sep 2026 06:57:36 -0500 Subject: [PATCH] py: in-place editing (clawhdf5.File(path, 'r+')) through FileEditor clawhdf5.File(path, 'r+') (and 'a' on an existing file) holds a FileEditor, and with it the file's exclusive lock, until close(): - ds[key] = value: h5py's keys and broadcasting (numpy's rules for slices and integers with extra leading 1-axes allowed; the exact shape for an index list, a scalar only where h5py expands it). Arrays are converted as libhdf5 converts them in native byte order (integers saturate, floats truncate toward zero and clip, integers go into h5py's bool enum by value); other values through numpy.asarray(value, dtype=ds.dtype), as h5py does. NaN into an integer dataset is a ValueError instead of libhdf5's arbitrary value. The value preparation is a small Python module compiled into the extension (src/edit_helpers.py). - ds.resize(shape) / ds.resize(n, axis=k) with h5py's argument rules. - attrs[name] = value, attrs.create(name, data, shape, dtype), attrs.modify: numeric, bool, complex, bytes and str data of any shape, with h5py's HDF5 types; str is stored as fixed-length UTF-8 (the editor cannot write variable-length strings). - File.mode, File.flush(), Dataset.chunks. Each edit runs with the GIL released under the file handle's write lock (no read sees a half-written edit), then the file is reopened; datasets and attrs objects re-read their shape and attributes when the handle's edit generation moved. What the editor cannot do is NotImplementedError before anything is written: deleting attributes or objects, creating datasets or groups, compound fields by name, variable-length data, and FileEditor's own limits. Where libhdf5 2.0 (h5py 3.16) converts inconsistently -- its soft conversions in non-native byte order (a float in (-1, 0) becomes the integer minimum, same-size unsigned->signed wraps) and native casts that are undefined in C (half floats into unsigned, float(max) rounded up) -- clawhdf5 saturates as libhdf5's native path does; listed in docs/known-issues.md. Tests (tests/test_edit.py): every edit applied by h5py and by clawhdf5 to copies of the same file and both read back through h5py after each edit, on h5py files (libver earliest, v114, latest) and a clawhdf5 file: a fixed sequence over every chunk index kind, compact/contiguous/gzip layouts and numeric, bool, enum, complex, string and compound types, 16 random sequences of 40 edits, and a numeric conversion matrix; a refused edit must be refused by both and leave the file unchanged. Also dense attributes, locking, objects seeing edits, readers racing a writer, and h5dump (plus h5rs check in ci-test.sh) on every edited file. The read-vs-h5py suite also runs on a file opened 'r+'. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 42 + README.md | 19 +- crates/clawhdf5-py/README.md | 41 +- crates/clawhdf5-py/src/attrs.rs | 207 +++-- crates/clawhdf5-py/src/dataset.rs | 212 ++++- crates/clawhdf5-py/src/edit.rs | 387 +++++++++ crates/clawhdf5-py/src/edit_helpers.py | 175 +++++ crates/clawhdf5-py/src/file.rs | 52 +- crates/clawhdf5-py/src/group.rs | 15 +- crates/clawhdf5-py/src/handle.rs | 118 ++- crates/clawhdf5-py/src/lib.rs | 8 + crates/clawhdf5-py/tests/test_edit.py | 742 ++++++++++++++++++ crates/clawhdf5-py/tests/test_read_vs_h5py.py | 13 +- docs/known-issues.md | 44 ++ scripts/ci-test.sh | 5 +- 15 files changed, 1978 insertions(+), 102 deletions(-) create mode 100644 crates/clawhdf5-py/src/edit.rs create mode 100644 crates/clawhdf5-py/src/edit_helpers.py create mode 100644 crates/clawhdf5-py/tests/test_edit.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 6b153b2..d37b2cf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,48 @@ ## Unreleased +### Python bindings: in-place editing (2026-09-27) +- **`clawhdf5.File(path, 'r+')`** (and `'a'` on an existing file) opens a + file for editing through `clawhdf5::FileEditor`, holding its exclusive + lock until `close()`: + - `ds[key] = value` with h5py's keys (integers, slices with steps, `...`, + one increasing index list) and broadcasting (numpy's rules for slices + and integers, allowing extra leading length-1 axes; the exact shape for + an index list, a scalar only where h5py expands it). A numpy array is + converted to the dataset's dtype as libhdf5 converts it in native byte + order (integers saturate; floats are truncated toward zero and clipped; + integers go into h5py's bool enum by value, as libhdf5 stores them); + other values through `numpy.asarray(value, dtype=ds.dtype)`, as h5py + does. NaN into an integer dataset is a `ValueError`. + - `ds.resize(shape)` / `ds.resize(n, axis=k)` with h5py's argument rules + and errors (`TypeError` for a dataset that is not chunked). + - `obj.attrs[name] = value`, `attrs.create(name, data, shape, dtype)`, + `attrs.modify`: numeric, bool, complex, bytes and `str` data of any + shape, stored with the HDF5 types h5py uses (bool as the `FALSE`/`TRUE` + enum, complex as the `r`/`i` compound), except that `str` becomes + fixed-length UTF-8. + - Every edit is written and synced before it returns, then the file is + reopened: datasets and attrs objects taken earlier see new shapes and + attributes, and reads on other threads wait while an edit is written. + - What the editor cannot do raises `NotImplementedError` and writes + nothing (deleting attributes or objects, creating datasets or groups, + compound fields by name, variable-length data, ...; + `docs/known-issues.md`). +- `File.mode`, `File.flush()` (a no-op), `Dataset.chunks`. +- Tests (`tests/test_edit.py`): each edit applied to two copies of a file, + by h5py and by clawhdf5, and both read back through h5py after every + edit, on files h5py writes with `libver` earliest, v114 and latest and on + a clawhdf5-written one: a fixed sequence over every chunk index kind, + compact/contiguous/gzip layouts and numeric, bool, enum, complex, string + and compound types, and 16 random sequences of 40 edits (writes, + resizes, attributes); where h5py refuses an edit clawhdf5 must refuse it + and leave its file unchanged. A matrix of every numeric source dtype into + every numeric dataset dtype at the edge values, dense attribute storage, + locking, objects seeing each other's edits, readers racing a writer + (never a partly written dataset). Every edited file must pass `h5dump` + and, in `ci-test.sh`, `h5rs check`. The read-vs-h5py suite also runs on + a file opened `'r+'`. + ### Python bindings: remote files (2026-09-27) - **`clawhdf5.File(url)`** opens `http://` URLs (and `https://`, `s3://`, `gs://`, `az://` in a wheel built with the `https`, `s3`, `gcs`, diff --git a/README.md b/README.md index dccee48..6d8f2a6 100644 --- a/README.md +++ b/README.md @@ -542,6 +542,22 @@ f = clawhdf5.File.open_url("http://data.example.org/run42.h5", block_size=256 * headers={"Authorization": "Bearer ..."}) ``` +An existing file opened with `"r+"` is edited in place (through +`clawhdf5::FileEditor`), with h5py's indexing, broadcasting and numeric +conversion; each edit is on disk when the statement returns: + +```python +with clawhdf5.File("data.h5", "r+") as f: + f["group/temperatures"][100:200, ::4] = 0.0 + f["series"].resize(5000, axis=0) # chunked datasets, within maxshape + f["series"][4000:] = new_values + f["group"].attrs["calibrated"] = True +``` + +Creating or deleting datasets, groups and attributes in an existing file is +not supported (`NotImplementedError`); limits are in +[known issues](docs/known-issues.md). + The default build reads `http://` URLs only; build with `maturin develop --release --features https` (rustls with ring, which compiles C) for `https://`, and `--features s3` (or `gcs`, `azure`) for @@ -561,7 +577,8 @@ non-default fill value (`docs/known-issues.md`). An index list is read one group of neighbouring chunks at a time. Writing (`File(path, "w")`, `create_dataset`, `create_group`, `attrs[...] =`) covers `float64`, `float32`, `int64`, `int32` and `uint8` arrays. The tests -in `crates/clawhdf5-py/tests` compare every read with h5py; run them with +in `crates/clawhdf5-py/tests` compare every read and every in-place edit +with h5py; run them with `pip install pytest h5py && pytest crates/clawhdf5-py/tests`. ### Agent Memory diff --git a/crates/clawhdf5-py/README.md b/crates/clawhdf5-py/README.md index 16e4169..50b0d17 100644 --- a/crates/clawhdf5-py/README.md +++ b/crates/clawhdf5-py/README.md @@ -98,6 +98,41 @@ chunks=..., compression="gzip")`, `create_group` and `attrs[...] = ...` writes `float64`, `float32`, `int64`, `int32` and `uint8` arrays; the file is written on `close()`. +## Editing a file in place + +`clawhdf5.File(path, "r+")` (or `"a"` on an existing file) edits the file +where it is, through clawhdf5's `FileEditor`; the file is locked until +`close()`, and every edit is written and synced before the statement +returns. + +```python +with clawhdf5.File("data.h5", "r+") as f: + ds = f["grid"] + ds[10:20, ::2] = 0 # h5py keys and broadcasting + ds[[1, 4, 7], 3] = [1.5, 2.5, 3.5] # one index list: exact shape + f["series"].resize((5000, 3)) # or .resize(5000, axis=0) + f["series"].attrs["units"] = "K" + f.attrs.create("version", 2, dtype="u1") +``` + +- Values: a numpy array is converted to the dataset's dtype as libhdf5 + converts it (integers saturate at the target's limits; floats are + truncated toward zero and clipped); anything else goes through + `numpy.asarray(value, dtype=ds.dtype)`, as in h5py. Writing NaN into an + integer dataset raises `ValueError` (libhdf5 would store an arbitrary + value). A few libhdf5 edge cases differ on purpose; see + `docs/known-issues.md`. +- Shapes: `ds.resize` grows or shrinks chunked datasets within their + `maxshape`, as h5py; datasets and `attrs` objects taken before an edit + see its result. +- Attributes: numeric, bool, complex, bytes and `str` data of any shape. + `str` is stored as a fixed-length UTF-8 string (h5py stores a + variable-length one), so h5py reads it back as `bytes`. +- Not supported (`NotImplementedError`, nothing written): creating or + deleting datasets, groups and attributes, writing compound fields by + name, variable-length data, HDF5 array types, and whatever + `FileEditor` refuses (listed in `docs/known-issues.md`). + ## Tests ```bash @@ -108,8 +143,10 @@ pytest crates/clawhdf5-py/tests `tests/test_read_vs_h5py.py` compares every read with h5py on a file h5py writes, opened locally and over HTTP (an in-process range server, `tests/conftest.py`); `tests/test_remote.py` checks remote reads (requests, -failures, the GIL). `scripts/ci-test.sh` builds the wheel and runs these -in CI. +failures, the GIL); `tests/test_edit.py` applies every edit through h5py and +clawhdf5 to copies of a file and compares them through h5py (and `h5dump`, +and `h5rs check` when `CLAWHDF5_H5RS` names it). `scripts/ci-test.sh` builds +the wheel and runs these in CI. ## License diff --git a/crates/clawhdf5-py/src/attrs.rs b/crates/clawhdf5-py/src/attrs.rs index 615214b..5b3f365 100644 --- a/crates/clawhdf5-py/src/attrs.rs +++ b/crates/clawhdf5-py/src/attrs.rs @@ -1,23 +1,62 @@ //! PyAttrs — dict-like access to HDF5 attributes. -use std::sync::{Arc, Mutex}; +use std::sync::{Arc, Mutex, PoisonError}; use clawhdf5_format::attribute::AttributeMessage; -use pyo3::exceptions::{PyKeyError, PyTypeError, PyValueError}; +use pyo3::exceptions::{PyKeyError, PyNotImplementedError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyList, PyTuple}; use crate::convert::{Converter, Elements, resolve_vl}; use crate::handle::Handle; -use crate::{OwnedAttrValue, PyEmpty, attr_value_to_py, node, py_to_attr_value}; +use crate::{OwnedAttrValue, PyEmpty, attr_value_to_py, edit, node, py_to_attr_value}; + +/// The attributes of an object in a file opened for reading (or editing). +struct ReadAttrs { + handle: Arc, + addr: u64, + path: String, + /// Sorted by name, with the file generation they were read at: an edit + /// (`attrs[name] = value`, here or through another handle on the same + /// object) makes them re-read. + cache: Mutex<(u64, Arc>)>, +} + +impl ReadAttrs { + fn current(&self, py: Python<'_>) -> PyResult>> { + let generation = self.handle.generation(); + { + let cached = self.cache.lock().unwrap_or_else(PoisonError::into_inner); + if cached.0 == generation { + return Ok(Arc::clone(&cached.1)); + } + } + let (addr, path) = (self.addr, &self.path); + let attrs = Arc::new(self.handle.with(py, |f| node::attributes(f, addr, path))?); + *self.cache.lock().unwrap_or_else(PoisonError::into_inner) = + (generation, Arc::clone(&attrs)); + Ok(attrs) + } + + fn check_writable(&self) -> PyResult<()> { + if self.handle.is_writable() { + return Ok(()); + } + Err(PyErr::new::( + "cannot set attributes on a read-only file (open it with mode 'r+')", + )) + } + + fn set(&self, py: Python<'_>, name: &str, value: clawhdf5_rs::AttrValue) -> PyResult<()> { + self.check_writable()?; + let path = node::name(&self.path); + self.handle.edit(py, |ed| ed.set_attr(&path, name, &value)) + } +} /// Backing storage for attributes. enum AttrsInner { - /// Attributes of an object in a file opened for reading, sorted by name. - Read { - handle: Arc, - attrs: Vec, - }, + Read(ReadAttrs), /// Writable attribute list shared with a parent (PyFile or PyGroup). Write(Arc>>), } @@ -27,8 +66,11 @@ enum AttrsInner { /// In read mode, values are what h5py returns: numpy scalars for scalar /// attributes, numpy arrays otherwise, `str` for variable-length strings, /// `numpy.bytes_` for fixed-length ones, and `Empty` for a null dataspace. -/// In write mode, attributes set here are accumulated and written when -/// the parent file is closed. +/// In a file opened with `'r+'`, `attrs[name] = value` adds or replaces an +/// attribute in the file at once (as h5py stores it, except that `str` +/// values become fixed-length UTF-8 strings). In write mode (`'w'`), +/// attributes set here are accumulated and written when the parent file is +/// closed. #[pyclass(name = "Attrs")] pub struct PyAttrs { inner: AttrsInner, @@ -43,9 +85,15 @@ impl PyAttrs { addr: u64, path: &str, ) -> PyResult { - let attrs = handle.with(py, |f| node::attributes(f, addr, path))?; + let generation = handle.generation(); + let attrs = Arc::new(handle.with(py, |f| node::attributes(f, addr, path))?); Ok(Self { - inner: AttrsInner::Read { handle, attrs }, + inner: AttrsInner::Read(ReadAttrs { + handle, + addr, + path: path.to_string(), + cache: Mutex::new((generation, attrs)), + }), }) } @@ -55,14 +103,48 @@ impl PyAttrs { inner: AttrsInner::Write(store), } } + + fn set_value( + &self, + py: Python<'_>, + key: &str, + value: &Bound<'_, PyAny>, + dtype: Option<&Bound<'_, PyAny>>, + shape: Option<&Bound<'_, PyAny>>, + ) -> PyResult<()> { + match &self.inner { + AttrsInner::Read(r) => { + r.check_writable()?; + let value = edit::attr_value(py, value, dtype, shape)?; + r.set(py, key, value) + } + AttrsInner::Write(store) => { + if dtype.is_some() || shape.is_some() { + return Err(PyNotImplementedError::new_err( + "attrs.create with a dtype or shape is only supported in a file opened \ + with 'r+'", + )); + } + let owned = py_to_attr_value(value)?; + let mut guard = store.lock().unwrap(); + // Replace existing key if present. + if let Some(entry) = guard.iter_mut().find(|(k, _)| k == key) { + entry.1 = owned; + } else { + guard.push((key.to_string(), owned)); + } + Ok(()) + } + } + } } #[pymethods] impl PyAttrs { fn __getitem__(&self, py: Python<'_>, key: &str) -> PyResult> { match &self.inner { - AttrsInner::Read { handle, attrs } => match attrs.iter().find(|a| a.name == key) { - Some(attr) => Ok(attr_to_py(py, handle, attr)?.unbind()), + AttrsInner::Read(r) => match r.current(py)?.iter().find(|a| a.name == key) { + Some(attr) => Ok(attr_to_py(py, &r.handle, attr)?.unbind()), None => Err(PyKeyError::new_err(format!( "Can't open attribute (can't locate attribute: '{key}')" ))), @@ -80,36 +162,63 @@ impl PyAttrs { } } - fn __setitem__(&self, key: &str, value: &Bound<'_, PyAny>) -> PyResult<()> { + /// `attrs[name] = value`. In a file opened with `'r+'` this writes the + /// attribute (numeric, bool, complex, bytes and str data, any shape) + /// into the file before returning; see the class docs. + fn __setitem__(&self, py: Python<'_>, key: &str, value: &Bound<'_, PyAny>) -> PyResult<()> { + self.set_value(py, key, value, None, None) + } + + /// Deleting attributes is not supported in a file (the in-place editor + /// cannot remove them); in write mode it removes a pending attribute. + fn __delitem__(&self, key: &str) -> PyResult<()> { match &self.inner { - AttrsInner::Read { .. } => Err(PyErr::new::( - "cannot set attributes on a read-only file", - )), + AttrsInner::Read(_) => Err(PyNotImplementedError::new_err(format!( + "cannot delete attribute '{key}': deleting attributes is not supported by \ + clawhdf5's in-place editor" + ))), AttrsInner::Write(store) => { - let owned = py_to_attr_value(value)?; let mut guard = store.lock().unwrap(); - // Replace existing key if present. - if let Some(entry) = guard.iter_mut().find(|(k, _)| k == key) { - entry.1 = owned; - } else { - guard.push((key.to_string(), owned)); + let before = guard.len(); + guard.retain(|(k, _)| k != key); + if guard.len() == before { + return Err(PyKeyError::new_err(key.to_string())); } Ok(()) } } } - fn __len__(&self) -> usize { + /// h5py's `attrs.create(name, data, shape=None, dtype=None)`: `data` + /// converted to `dtype` and reshaped to `shape` first. + #[pyo3(signature = (name, data, shape=None, dtype=None))] + fn create( + &self, + py: Python<'_>, + name: &str, + data: &Bound<'_, PyAny>, + shape: Option<&Bound<'_, PyAny>>, + dtype: Option<&Bound<'_, PyAny>>, + ) -> PyResult<()> { + self.set_value(py, name, data, dtype, shape) + } + + /// h5py's `attrs.modify(name, value)`: same as `attrs[name] = value`. + fn modify(&self, py: Python<'_>, name: &str, value: &Bound<'_, PyAny>) -> PyResult<()> { + self.set_value(py, name, value, None, None) + } + + fn __len__(&self, py: Python<'_>) -> PyResult { match &self.inner { - AttrsInner::Read { attrs, .. } => attrs.len(), - AttrsInner::Write(store) => store.lock().unwrap().len(), + AttrsInner::Read(r) => Ok(r.current(py)?.len()), + AttrsInner::Write(store) => Ok(store.lock().unwrap().len()), } } - fn __contains__(&self, key: &str) -> bool { + fn __contains__(&self, py: Python<'_>, key: &str) -> PyResult { match &self.inner { - AttrsInner::Read { attrs, .. } => attrs.iter().any(|a| a.name == key), - AttrsInner::Write(store) => store.lock().unwrap().iter().any(|(k, _)| k == key), + AttrsInner::Read(r) => Ok(r.current(py)?.iter().any(|a| a.name == key)), + AttrsInner::Write(store) => Ok(store.lock().unwrap().iter().any(|(k, _)| k == key)), } } @@ -119,15 +228,17 @@ impl PyAttrs { Ok(iter) } - fn __repr__(&self) -> String { - let n = self.__len__(); - format!("") + fn __repr__(&self, py: Python<'_>) -> String { + match self.__len__(py) { + Ok(n) => format!(""), + Err(_) => "".to_string(), + } } /// The value of `key`, or `default` if there is no such attribute. #[pyo3(signature = (key, default=None))] fn get(&self, py: Python<'_>, key: &str, default: Option>) -> PyResult> { - if self.__contains__(key) { + if self.__contains__(py, key)? { self.__getitem__(py, key) } else { Ok(default.unwrap_or_else(|| py.None())) @@ -137,7 +248,7 @@ impl PyAttrs { /// Return attribute names as a list. fn keys(&self, py: Python<'_>) -> PyResult> { let names: Vec = match &self.inner { - AttrsInner::Read { attrs, .. } => attrs.iter().map(|a| a.name.clone()).collect(), + AttrsInner::Read(r) => r.current(py)?.iter().map(|a| a.name.clone()).collect(), AttrsInner::Write(store) => store .lock() .unwrap() @@ -152,9 +263,10 @@ impl PyAttrs { /// Return attribute values as a list. fn values(&self, py: Python<'_>) -> PyResult> { let vals: Vec> = match &self.inner { - AttrsInner::Read { handle, attrs } => attrs + AttrsInner::Read(r) => r + .current(py)? .iter() - .map(|a| attr_to_py(py, handle, a).map(Bound::unbind)) + .map(|a| attr_to_py(py, &r.handle, a).map(Bound::unbind)) .collect::>()?, AttrsInner::Write(store) => store .lock() @@ -173,9 +285,10 @@ impl PyAttrs { /// Return attribute (key, value) pairs as a list of tuples. fn items(&self, py: Python<'_>) -> PyResult> { let pairs: Vec<(String, Py)> = match &self.inner { - AttrsInner::Read { handle, attrs } => attrs + AttrsInner::Read(r) => r + .current(py)? .iter() - .map(|a| Ok((a.name.clone(), attr_to_py(py, handle, a)?.unbind()))) + .map(|a| Ok((a.name.clone(), attr_to_py(py, &r.handle, a)?.unbind()))) .collect::>()?, AttrsInner::Write(store) => store .lock() @@ -248,19 +361,3 @@ fn prefix_err(py: Python<'_>, name: &str, e: PyErr) -> PyErr { PyValueError::new_err(msg) } } - -#[cfg(test)] -mod tests { - use super::*; - - #[test] - fn write_attrs_len() { - let store = Arc::new(Mutex::new(Vec::new())); - store - .lock() - .unwrap() - .push(("key".into(), OwnedAttrValue::I64(99))); - let attrs = PyAttrs::from_write(store); - assert_eq!(attrs.__len__(), 1); - } -} diff --git a/crates/clawhdf5-py/src/dataset.rs b/crates/clawhdf5-py/src/dataset.rs index 24c9b51..d837f73 100644 --- a/crates/clawhdf5-py/src/dataset.rs +++ b/crates/clawhdf5-py/src/dataset.rs @@ -9,12 +9,12 @@ //! Python threads reading the same or different datasets run in parallel, //! and a remote file's network reads never hold the GIL. -use std::sync::Arc; +use std::sync::{Arc, Mutex, PoisonError}; use clawhdf5_format::datatype::Datatype; use clawhdf5_format::object_header::ObjectHeader; use clawhdf5_rs::File; -use pyo3::exceptions::{PyTypeError, PyValueError}; +use pyo3::exceptions::{PyNotImplementedError, PyOSError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyList, PyTuple}; @@ -22,7 +22,7 @@ use crate::attrs::PyAttrs; use crate::convert::{Converter, Elements, VlError, resolve_vl}; use crate::handle::Handle; use crate::select::{self, Plan}; -use crate::{PyEmpty, node, to_py_err}; +use crate::{PyEmpty, edit, node, to_py_err}; /// What opening a dataset reads from the file (without the GIL). pub(crate) struct DatasetMeta { @@ -67,8 +67,9 @@ pub struct PyDataset { /// Where the dataset's object header is: reads open it from here rather /// than resolve `path` again. addr: u64, - /// `None` for a dataset with a null dataspace (h5py's `Empty`). - shape: Option>, + /// The shape (`None` for a null dataspace, h5py's `Empty`), with the + /// file generation it was read at: an edit (a resize) may change it. + shape: Mutex<(u64, Option>)>, /// The chunk shape, for a chunked dataset. chunks: Option>, datatype: Datatype, @@ -84,13 +85,14 @@ impl PyDataset { addr: u64, meta: DatasetMeta, ) -> Self { + let generation = handle.generation(); let conv = crate::no_panic(|| Converter::new(py, &meta.datatype, handle.offset_size)) .map_err(|e| e.value(py).to_string()); Self { handle, path, addr, - shape: meta.shape, + shape: Mutex::new((generation, meta.shape)), chunks: meta.chunks, datatype: meta.datatype, conv, @@ -103,10 +105,54 @@ impl PyDataset { .map_err(|msg| PyTypeError::new_err(format!("{}: {msg}", node::name(&self.path)))) } + /// The current shape: the one read at open, or re-read after an edit. + fn dims(&self, py: Python<'_>) -> PyResult>> { + let generation = self.handle.generation(); + { + let cached = self.shape.lock().unwrap_or_else(PoisonError::into_inner); + if cached.0 == generation { + return Ok(cached.1.clone()); + } + } + let addr = self.addr; + let null = self + .shape + .lock() + .unwrap_or_else(PoisonError::into_inner) + .1 + .is_none(); + let shape = if null { + None + } else { + Some(self.handle.with(py, |f| { + f.dataset_at(addr) + .and_then(|ds| ds.shape()) + .map_err(to_py_err) + })?) + }; + *self.shape.lock().unwrap_or_else(PoisonError::into_inner) = (generation, shape.clone()); + Ok(shape) + } + + fn check_writable(&self) -> PyResult<()> { + if self.handle.is_writable() { + Ok(()) + } else { + Err(PyOSError::new_err(format!( + "{}: the file is open read-only; open it with mode 'r+' to change it", + node::name(&self.path) + ))) + } + } + /// Read the selection described by `plan` into a numpy array. - fn read_plan<'py>(&self, py: Python<'py>, plan: &Plan) -> PyResult> { + fn read_plan<'py>( + &self, + py: Python<'py>, + plan: &Plan, + dims: &[u64], + ) -> PyResult> { let conv = self.converter()?; - let dims = self.shape.as_deref().unwrap_or(&[]); let out_shape = plan.out_shape(); let arr = if plan.is_empty() { @@ -272,7 +318,7 @@ impl PyDataset { /// The shape of the dataset (`None` for an empty/null dataspace). #[getter] fn shape<'py>(&self, py: Python<'py>) -> PyResult> { - match &self.shape { + match self.dims(py)? { Some(s) => Ok(PyTuple::new(py, s)?.into_any()), None => Ok(py.None().into_bound(py)), } @@ -281,7 +327,7 @@ impl PyDataset { /// The maximum shape (`None` per unlimited dimension), like h5py. #[getter] fn maxshape<'py>(&self, py: Python<'py>) -> PyResult> { - let Some(shape) = &self.shape else { + let Some(shape) = self.dims(py)? else { return Ok(py.None().into_bound(py)); }; let addr = self.addr; @@ -292,7 +338,7 @@ impl PyDataset { .and_then(|ds| ds.max_dimensions()) .map_err(to_py_err) })? - .unwrap_or_else(|| shape.clone()); + .unwrap_or(shape); let items: Vec> = max .into_iter() .map(|d| (d != u64::MAX).then_some(d)) @@ -300,6 +346,15 @@ impl PyDataset { Ok(PyTuple::new(py, items)?.into_any()) } + /// The chunk shape, or `None` for a dataset that is not chunked. + #[getter] + fn chunks<'py>(&self, py: Python<'py>) -> PyResult> { + match &self.chunks { + Some(c) => Ok(PyTuple::new(py, c)?.into_any()), + None => Ok(py.None().into_bound(py)), + } + } + /// The dataset's numpy dtype, as h5py reports it. #[getter] fn dtype<'py>(&self, py: Python<'py>) -> PyResult> { @@ -307,14 +362,14 @@ impl PyDataset { } #[getter] - fn ndim(&self) -> usize { - self.shape.as_ref().map_or(0, Vec::len) + fn ndim(&self, py: Python<'_>) -> PyResult { + Ok(self.dims(py)?.map_or(0, |s| s.len())) } /// Number of elements (`None` for an empty/null dataspace, as h5py). #[getter] - fn size(&self) -> Option { - self.shape.as_ref().map(|s| s.iter().product()) + fn size(&self, py: Python<'_>) -> PyResult> { + Ok(self.dims(py)?.map(|s| s.iter().product())) } /// The dataset's full name, e.g. `/group/data`. @@ -323,7 +378,8 @@ impl PyDataset { node::name(&self.path) } - /// The dataset's attributes (read-only, dict-like). + /// The dataset's attributes (dict-like; writable in a file opened with + /// `'r+'`). #[getter] fn attrs(&self, py: Python<'_>) -> PyResult { PyAttrs::read(py, Arc::clone(&self.handle), self.addr, &self.path) @@ -338,7 +394,7 @@ impl PyDataset { py: Python<'py>, key: &Bound<'py, PyAny>, ) -> PyResult> { - let Some(dims) = &self.shape else { + let Some(dims) = self.dims(py)? else { let is_empty_tuple = key.cast::().is_ok_and(|t| t.is_empty()); let is_ellipsis = key.is_instance_of::(); if is_empty_tuple || is_ellipsis { @@ -347,8 +403,109 @@ impl PyDataset { } return Err(PyValueError::new_err("Empty datasets cannot be sliced")); }; - let plan = select::parse(key, dims)?; - self.read_plan(py, &plan) + let plan = select::parse(key, &dims)?; + self.read_plan(py, &plan, &dims) + } + + /// Write with h5py indexing (file opened with `'r+'`): `ds[key] = value`. + /// + /// The key is what `ds[key]` reads (without compound field names). The + /// value is converted to the dataset's dtype as h5py converts it (a + /// numpy array as libhdf5 does, clipping out-of-range numbers; anything + /// else through `numpy.asarray(value, dtype=ds.dtype)`), and broadcast + /// to the selection as h5py broadcasts. The edit is written and synced + /// before this returns; what the in-place editor cannot write raises + /// `NotImplementedError` and leaves the file as it was. + fn __setitem__( + &self, + py: Python<'_>, + key: &Bound<'_, PyAny>, + value: &Bound<'_, PyAny>, + ) -> PyResult<()> { + self.check_writable()?; + let Some(dims) = self.dims(py)? else { + return Err(PyNotImplementedError::new_err( + "writing to an empty (null dataspace) dataset is not supported", + )); + }; + let plan = select::parse(key, &dims)?; + if !plan.fields.is_empty() { + return Err(PyNotImplementedError::new_err( + "writing compound fields by name is not supported by clawhdf5's in-place editor; \ + write whole elements", + )); + } + let category = edit::category(&self.datatype)?; + let conv = self.converter()?; + let bytes = edit::dataset_bytes( + py, + value, + conv.dtype.bind(py), + category, + &plan, + self.chunks.as_deref(), + )?; + if plan.is_empty() { + return Ok(()); + } + let sel = edit::selection(&plan, &dims)?; + let path = node::name(&self.path); + self.handle + .edit(py, |ed| ed.write_selection(&path, &sel, &bytes)) + } + + /// Change the dataset's shape (file opened with `'r+'`), as h5py's + /// `Dataset.resize`: `ds.resize((100, 20))`, or `ds.resize(100, axis=0)`. + /// Only chunked datasets, within their maximum shape; new elements read + /// as the fill value. + #[pyo3(signature = (size, axis=None))] + fn resize(&self, py: Python<'_>, size: &Bound<'_, PyAny>, axis: Option) -> PyResult<()> { + self.check_writable()?; + let Some(dims) = self.dims(py)? else { + return Err(PyTypeError::new_err("Empty datasets cannot be resized")); + }; + if self.chunks.is_none() { + return Err(PyTypeError::new_err("Only chunked datasets can be resized")); + } + let shape: Vec = match axis { + Some(axis) => { + let rank = dims.len(); + let a = usize::try_from(axis) + .ok() + .filter(|&a| a < rank) + .ok_or_else(|| { + PyValueError::new_err(format!( + "Invalid axis (0 to {} allowed)", + rank.saturating_sub(1) + )) + })?; + let n: u64 = size.extract().map_err(|_| { + PyTypeError::new_err("Argument must be a single int if axis is specified") + })?; + let mut s = dims.clone(); + s[a] = n; + s + } + // As h5py: without `axis` the size is a sequence (`tuple(size)`). + None => size.extract().map_err(|_| { + PyTypeError::new_err(format!( + "'{}' object is not iterable", + size.get_type() + .name() + .map(|n| n.to_string()) + .unwrap_or_default() + )) + })?, + }; + if shape.len() != dims.len() { + return Err(PyValueError::new_err(format!( + "new shape {shape:?} has {} dimensions, the dataset {}", + shape.len(), + dims.len() + ))); + } + let path = node::name(&self.path); + self.handle.edit(py, |ed| ed.resize(&path, &shape)) } /// `numpy.asarray(ds)` reads the whole dataset. @@ -360,20 +517,20 @@ impl PyDataset { copy: Option, ) -> PyResult> { let _ = copy; // every read is a fresh array - let Some(dims) = &self.shape else { + let Some(dims) = self.dims(py)? else { return Err(PyValueError::new_err("an empty dataset has no array value")); }; let ellipsis = pyo3::types::PyEllipsis::get(py).to_owned().into_any(); - let plan = select::parse(&ellipsis, dims)?; - let arr = self.read_plan(py, &plan)?; + let plan = select::parse(&ellipsis, &dims)?; + let arr = self.read_plan(py, &plan, &dims)?; match dtype { Some(dt) => arr.call_method1("astype", (dt,)), None => Ok(arr), } } - fn __len__(&self) -> PyResult { - match self.shape.as_deref() { + fn __len__(&self, py: Python<'_>) -> PyResult { + match self.dims(py)?.as_deref() { Some([first, ..]) => Ok(*first as usize), _ => Err(PyTypeError::new_err( "Attempt to take len() of scalar dataset", @@ -391,9 +548,10 @@ impl PyDataset { .unwrap_or_default(), Err(_) => format!("{:?}", self.datatype), }; - let shape = match &self.shape { - Some(s) => format!("{s:?}"), - None => "None".to_string(), + let shape = match self.dims(py) { + Ok(Some(s)) => format!("{s:?}"), + Ok(None) => "None".to_string(), + Err(_) => "?".to_string(), }; format!( "", diff --git a/crates/clawhdf5-py/src/edit.rs b/crates/clawhdf5-py/src/edit.rs new file mode 100644 index 0000000..05dd536 --- /dev/null +++ b/crates/clawhdf5-py/src/edit.rs @@ -0,0 +1,387 @@ +//! In-place editing (`clawhdf5.File(path, 'r+')`) through `FileEditor`: +//! turning what Python assigns into the bytes, selections and attribute +//! values the editor takes. +//! +//! Value conversion follows h5py (see `edit_helpers.py`, run inside the +//! extension module); what `FileEditor` cannot do is `NotImplementedError` +//! before anything is written. + +use std::ffi::CString; + +use clawhdf5_format::datatype::{ + CharacterSet, CompoundMember, Datatype, DatatypeByteOrder, EnumMember, StringPadding, +}; +use clawhdf5_format::selection::Selection; +use clawhdf5_rs::AttrValue; +use pyo3::exceptions::{PyNotImplementedError, PyTypeError}; +use pyo3::prelude::*; +use pyo3::sync::PyOnceLock; +use pyo3::types::{PyBytes, PyModule, PyTuple}; + +use crate::select::{Axis, Plan}; + +/// The helper module, compiled once. +pub(crate) fn helpers(py: Python<'_>) -> PyResult<&Bound<'_, PyModule>> { + static HELPERS: PyOnceLock> = PyOnceLock::new(); + let module = HELPERS.get_or_try_init(py, || -> PyResult> { + let code = CString::new(include_str!("edit_helpers.py")) + .map_err(|e| PyTypeError::new_err(e.to_string()))?; + Ok(PyModule::from_code( + py, + &code, + c"clawhdf5/edit_helpers.py", + c"clawhdf5._edit_helpers", + )? + .unbind()) + })?; + Ok(module.bind(py)) +} + +fn not_implemented(what: impl std::fmt::Display) -> PyErr { + PyNotImplementedError::new_err(format!( + "{what} is not supported by clawhdf5's in-place editor" + )) +} + +/// How values for a dataset of type `dt` are converted (a category of +/// `edit_helpers._convert_array`), or why they cannot be written. +pub(crate) fn category(dt: &Datatype) -> PyResult<&'static str> { + match dt { + Datatype::FixedPoint { .. } => Ok("int"), + Datatype::FloatingPoint { .. } => Ok("float"), + Datatype::Enumeration { + base_type, members, .. + } => { + let is_bool = base_type.type_size() == 1 + && members.len() == 2 + && members + .iter() + .any(|m| m.name == "FALSE" && m.value.first() == Some(&0)) + && members + .iter() + .any(|m| m.name == "TRUE" && m.value.first() == Some(&1)); + Ok(if is_bool { "bool" } else { "enum" }) + } + Datatype::String { + padding: StringPadding::NullPad, + .. + } => Ok("string"), + Datatype::String { padding, .. } => Err(not_implemented(format!( + "writing fixed-length strings padded {padding:?} (libhdf5 converts them \ + differently from numpy)" + ))), + Datatype::Compound { size, members } => { + if is_complex(*size, members) { + return Ok("complex"); + } + check_exact(dt)?; + Ok("exact") + } + Datatype::Opaque { .. } => Ok("exact"), + Datatype::Array { .. } => Err(not_implemented("writing HDF5 array-type elements")), + Datatype::VariableLength { .. } => Err(not_implemented("writing variable-length data")), + Datatype::Reference { .. } => Err(not_implemented("writing references")), + Datatype::BitField { .. } => Err(not_implemented("writing bitfields")), + Datatype::Time { .. } => Err(not_implemented("writing time values")), + } +} + +/// h5py's complex numbers: a compound of two identical floats `r`, `i`. +fn is_complex(size: u32, members: &[CompoundMember]) -> bool { + matches!(members, [r, i] if r.name == "r" && i.name == "i" + && r.datatype == i.datatype + && matches!(r.datatype, Datatype::FloatingPoint { size: fs, .. } + if r.byte_offset == 0 && i.byte_offset == u64::from(fs) && size == 2 * fs)) +} + +/// Compound members written byte for byte from the same numpy dtype: fine +/// unless libhdf5 would convert them on the way (strings padded other than +/// with NULs), or the editor cannot write them at all. +fn check_exact(dt: &Datatype) -> PyResult<()> { + match dt { + Datatype::Compound { members, .. } => { + members.iter().try_for_each(|m| check_exact(&m.datatype)) + } + Datatype::Array { base_type, .. } => check_exact(base_type), + Datatype::String { + padding: StringPadding::NullPad, + .. + } + | Datatype::FixedPoint { .. } + | Datatype::FloatingPoint { .. } + | Datatype::Enumeration { .. } + | Datatype::Opaque { .. } + | Datatype::BitField { .. } => Ok(()), + Datatype::String { .. } => Err(not_implemented( + "writing compounds with strings not padded with NULs", + )), + Datatype::VariableLength { .. } => Err(not_implemented( + "writing compounds with variable-length members", + )), + Datatype::Reference { .. } => Err(not_implemented("writing references")), + Datatype::Time { .. } => Err(not_implemented("writing time values")), + } +} + +/// Largest point selection an index-list write builds (one coordinate +/// vector per element). +const MAX_POINTS: usize = 1 << 22; + +/// The selection `plan` writes, whose elements are numbered as the value's +/// (row-major over the selection's shape). +pub(crate) fn selection(plan: &Plan, dims: &[u64]) -> PyResult { + if plan.axes.is_empty() { + return Ok(Selection::All); + } + if plan.list_axis().is_none() { + let (reads, _) = plan.reads(dims, None, 1); + return match <[_; 1]>::try_from(reads) { + Ok([read]) => Ok(read.sel), + Err(_) => Err(PyTypeError::new_err("internal error: several hyperslabs")), + }; + } + // An index list: the points, in the value's order. + let per_axis: Vec> = plan + .axes + .iter() + .map(|a| match a { + Axis::Index(i) => vec![*i], + Axis::Slice { start, step, count } => (0..*count).map(|k| start + k * step).collect(), + Axis::List(v) => v.clone(), + }) + .collect(); + let n = per_axis + .iter() + .try_fold(1usize, |acc, v| acc.checked_mul(v.len())) + .filter(|&n| n <= MAX_POINTS) + .ok_or_else(|| { + not_implemented(format!( + "an index-list write of more than {MAX_POINTS} elements (write it in slices)" + )) + })?; + let mut points = Vec::with_capacity(n); + let mut at = vec![0usize; per_axis.len()]; + for _ in 0..n { + points.push(at.iter().zip(&per_axis).map(|(&i, v)| v[i]).collect()); + for d in (0..at.len()).rev() { + at[d] += 1; + if at[d] < per_axis[d].len() { + break; + } + at[d] = 0; + } + } + Ok(Selection::Points(points)) +} + +/// The bytes to write for `value` under `plan`, in the dataset's dtype. +pub(crate) fn dataset_bytes( + py: Python<'_>, + value: &Bound<'_, PyAny>, + dtype: &Bound<'_, PyAny>, + category: &str, + plan: &Plan, + chunks: Option<&[u64]>, +) -> PyResult> { + let shape = PyTuple::new(py, plan.out_shape())?; + let fancy = plan.list_axis().is_some(); + let chunk_elems = chunks.map_or(0, |c| c.iter().fold(1u64, |a, &d| a.saturating_mul(d))); + let bytes = helpers(py)?.call_method1( + "dataset_values", + (value, dtype, category, shape, fancy, chunk_elems), + )?; + Ok(bytes.cast::()?.as_bytes().to_vec()) +} + +fn ieee_float(size: u32, byte_order: DatatypeByteOrder) -> Option { + let (exponent_location, exponent_size, mantissa_size, exponent_bias) = match size { + 2 => (10, 5, 10, 15), + 4 => (23, 8, 23, 127), + 8 => (52, 11, 52, 1023), + _ => return None, + }; + Some(Datatype::FloatingPoint { + size, + byte_order, + bit_offset: 0, + bit_precision: (size * 8) as u16, + exponent_location, + exponent_size, + mantissa_location: 0, + mantissa_size, + exponent_bias, + }) +} + +/// The HDF5 datatype h5py writes for a numpy dtype string (`'f8'`, `' Option { + let order = match dtype.as_bytes().first()? { + b'<' | b'|' | b'=' => DatatypeByteOrder::LittleEndian, + b'>' => DatatypeByteOrder::BigEndian, + _ => return None, + }; + let kind = dtype.as_bytes().get(1)?; + let size: u32 = dtype.get(2..)?.parse().ok()?; + match kind { + b'b' if size == 1 => Some(Datatype::Enumeration { + size: 1, + base_type: Box::new(Datatype::FixedPoint { + size: 1, + byte_order: DatatypeByteOrder::LittleEndian, + signed: true, + bit_offset: 0, + bit_precision: 8, + }), + members: vec![ + EnumMember { + name: "FALSE".into(), + value: vec![0], + }, + EnumMember { + name: "TRUE".into(), + value: vec![1], + }, + ], + }), + b'i' | b'u' if matches!(size, 1 | 2 | 4 | 8) => Some(Datatype::FixedPoint { + size, + byte_order: order, + signed: *kind == b'i', + bit_offset: 0, + bit_precision: (size * 8) as u16, + }), + b'f' => ieee_float(size, order), + b'c' => { + let part = ieee_float(size / 2, order)?; + Some(Datatype::Compound { + size, + members: vec![ + CompoundMember { + name: "r".into(), + byte_offset: 0, + datatype: part.clone(), + }, + CompoundMember { + name: "i".into(), + byte_offset: u64::from(size / 2), + datatype: part, + }, + ], + }) + } + b'S' if size > 0 => Some(Datatype::String { + size, + padding: StringPadding::NullPad, + charset: CharacterSet::Ascii, + }), + _ => None, + } +} + +/// An attribute value as h5py would store it (`attrs[name] = value`, or +/// `attrs.create(name, data, shape, dtype)`), except that `str` data is +/// stored as fixed-length UTF-8 strings (h5py stores variable-length ones, +/// which the editor cannot write). +pub(crate) fn attr_value( + py: Python<'_>, + value: &Bound<'_, PyAny>, + dtype: Option<&Bound<'_, PyAny>>, + shape: Option<&Bound<'_, PyAny>>, +) -> PyResult { + if value.is_instance_of::() { + return Err(not_implemented( + "writing an empty (null dataspace) attribute", + )); + } + let (kind, dt, dims, data): (String, String, Vec, Vec) = helpers(py)? + .call_method1("attr_value", (value, dtype, shape))? + .extract()?; + let datatype = if kind == "str" { + let size: u32 = dt + .parse() + .map_err(|_| PyTypeError::new_err("bad string size"))?; + Datatype::String { + size, + padding: StringPadding::NullPad, + charset: CharacterSet::Utf8, + } + } else { + datatype_of(&dt).ok_or_else(|| not_implemented(format!("an attribute of dtype {dt}")))? + }; + Ok(AttrValue::Raw { + datatype, + shape: dims, + data, + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn numpy_dtypes_map_to_h5py_types() { + assert!(matches!( + datatype_of("u2"), + Some(Datatype::FixedPoint { + size: 2, + signed: false, + byte_order: DatatypeByteOrder::BigEndian, + .. + }) + )); + assert!(matches!( + datatype_of(" {dst!r}") + + +def _to_int(arr, dtype): + """Integer target: libhdf5's saturating conversion.""" + info = np.iinfo(dtype) + kind = arr.dtype.kind + if kind == "b": + return arr.astype(dtype) + if kind in "iu": + src = np.iinfo(arr.dtype) + lo = max(info.min, src.min) + hi = min(info.max, src.max) + clipped = np.clip(arr, np.array(lo, arr.dtype), np.array(hi, arr.dtype)) + return clipped.astype(dtype) + if kind == "f": + if np.isnan(arr).any(): + raise ValueError( + "cannot write NaN to an integer dataset (libhdf5 would store an arbitrary value)" + ) + t = np.trunc(arr.astype(np.float64)) + # info.max + 1 and info.min are powers of two: exact as floats. + over = t >= float(info.max + 1) + under = t < float(info.min) + out = np.where(over | under, 0.0, t).astype(dtype) + out[over] = info.max + out[under] = info.min + return out + raise _no_path(arr.dtype, dtype) + + +def _convert_array(arr, dtype, category): + kind = arr.dtype.kind + if category in ("int", "enum"): + if arr.dtype == dtype and kind in "iu": + return arr + if category == "enum" and kind not in "iu": + raise _no_path(arr.dtype, dtype) + return _to_int(arr, dtype) + if category == "bool": + if kind == "b": + return arr.astype(dtype) + if kind in "iu": + # h5py's bool is an enum over int8; libhdf5 converts integers + # into it by value (saturating), not to FALSE/TRUE, so 3 is + # stored as 3. Keep those bytes: a view, not a cast. + return _to_int(arr, np.dtype("i1")).view(dtype) + raise _no_path(arr.dtype, dtype) + if category == "float": + if kind not in "biuf": + raise _no_path(arr.dtype, dtype) + with np.errstate(over="ignore", invalid="ignore"): + return arr.astype(dtype) + if category == "complex": + if kind != "c": + raise _no_path(arr.dtype, dtype) + with np.errstate(over="ignore", invalid="ignore"): + return arr.astype(dtype) + if category == "string": + if kind != "S": + raise _no_path(arr.dtype, dtype) + return arr.astype(dtype) + # "exact": compound and opaque types, written only from the same dtype. + if arr.dtype == dtype: + return arr + raise _no_path(arr.dtype, dtype) + + +def _convert_other(value, dtype, category): + if category == "string": + items = np.asarray(value, dtype=object) + if any(isinstance(x, str) for x in items.flat): + meta = dtype.metadata or {} + if meta.get("h5py_encoding") == "utf-8": + enc = [x.encode("utf-8") if isinstance(x, str) else x for x in items.flat] + return np.array(enc, dtype=dtype).reshape(items.shape) + return np.asarray(value, dtype=dtype) + + +def _broadcast(arr, shape, fancy, chunk_elems): + """h5py's broadcasting: numpy's rules against the selection's shape + (extra leading length-1 axes allowed) for slices and integers. For an + index list, the exact shape; a scalar only where h5py expands it to the + whole selection (a chunked dataset whose chunk holds at least as many + elements as the selection).""" + if arr.shape == shape: + return arr + if fancy: + size = int(np.prod(shape)) + if arr.ndim == 0 and ((chunk_elems > 0 and size <= chunk_elems) or len(shape) == 1): + return np.broadcast_to(arr, shape) + raise TypeError("Broadcasting is not supported for complex selections") + if arr.ndim == 0: + return np.broadcast_to(arr, shape) + err = TypeError(f"Can't broadcast {arr.shape} -> {shape}") + src = arr.shape + while len(src) > len(shape) and src[0] == 1: + src = src[1:] + if len(src) > len(shape): + raise err + try: + return np.broadcast_to(arr.reshape(src), shape) + except ValueError: + raise err from None + + +def dataset_values(value, dtype, category, shape, fancy, chunk_elems): + """The bytes to write for `value` under a selection of `shape`, as a + C-ordered array of the dataset's dtype.""" + if isinstance(value, np.ndarray): + arr = _convert_array(value, dtype, category) + else: + arr = _convert_other(value, dtype, category) + arr = _broadcast(arr, tuple(shape), fancy, chunk_elems) + return np.ascontiguousarray(arr, dtype=dtype).tobytes() + + +def attr_value(value, dtype=None, shape=None): + """(kind, dtype string, shape, bytes) for an attribute value, h5py's + `attrs[name] = value` / `attrs.create(name, data, shape, dtype)`: + + - "str": `str` data (h5py would store a variable-length string; this + stores a fixed-length UTF-8 string, which clawhdf5 can write); + the dtype string is the byte length of the longest element; + - "raw": a numeric, bool or bytes array, as numpy lays it out. + """ + if dtype is not None: + arr = np.asarray(value, dtype=dtype, order="C") + else: + arr = np.asarray(value, order="C") + if shape is not None: + arr = arr.reshape(shape) + kind = arr.dtype.kind + if kind == "O": + if arr.size and all(isinstance(x, str) for x in arr.flat): + kind = "U" + elif arr.size and all(isinstance(x, bytes) for x in arr.flat): + arr = arr.astype(bytes) + kind = "S" + else: + raise TypeError( + f"clawhdf5 cannot write an attribute of Python objects ({value!r:.60})" + ) + if kind == "U": + enc = [str(x).encode("utf-8") for x in arr.flat] + size = max([len(b) for b in enc] + [1]) + data = np.array(enc, dtype=f"S{size}").reshape(arr.shape) + return ("str", str(size), arr.shape, data.tobytes()) + if kind in "biufcS": + return ("raw", arr.dtype.str, arr.shape, np.ascontiguousarray(arr).tobytes()) + raise NotImplementedError( + f"clawhdf5 cannot write an attribute of dtype {arr.dtype} in place" + ) diff --git a/crates/clawhdf5-py/src/file.rs b/crates/clawhdf5-py/src/file.rs index 48d94c0..2764e61 100644 --- a/crates/clawhdf5-py/src/file.rs +++ b/crates/clawhdf5-py/src/file.rs @@ -5,7 +5,7 @@ use std::path::PathBuf; use std::sync::{Arc, Mutex}; use std::time::Duration; -use pyo3::exceptions::PyValueError; +use pyo3::exceptions::{PyNotImplementedError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyDict, PyList}; @@ -95,6 +95,18 @@ impl PyFile { } match mode { "r" => Ok(Self::from_handle(Handle::open_local(py, path)?, filename)), + "r+" => Ok(Self::from_handle( + Handle::open_editable(py, path)?, + filename, + )), + "a" if std::path::Path::new(path).exists() => Ok(Self::from_handle( + Handle::open_editable(py, path)?, + filename, + )), + "a" => Err(PyNotImplementedError::new_err(format!( + "mode 'a' on {path}, which does not exist: clawhdf5 can only edit an existing \ + file in place; create a new one with mode 'w'" + ))), "w" => Ok(Self { filename, inner: Some(FileInner::Write(WriteState { @@ -105,7 +117,7 @@ impl PyFile { })), }), other => Err(PyValueError::new_err(format!( - "unsupported mode '{other}'; expected 'r' or 'w'" + "unsupported mode '{other}'; expected 'r', 'r+', 'a' or 'w'" ))), } } @@ -226,7 +238,10 @@ impl PyFile { PyErr::new::("file is already closed") })?; match inner { - FileInner::Read(_) => Ok(()), + FileInner::Read(root) => { + root.handle.close(); + Ok(()) + } FileInner::Write(state) => finalize_write(state), } } @@ -289,6 +304,30 @@ impl PyFile { "/" } + /// `'r'` for a file opened read-only (a local file or a URL), `'r+'` + /// for one open for editing or writing, as h5py reports it. + #[getter] + fn mode(&self) -> PyResult<&'static str> { + match &self.inner { + Some(FileInner::Read(root)) if !root.handle.is_writable() => Ok("r"), + Some(_) => Ok("r+"), + None => Err(PyErr::new::( + "file is closed", + )), + } + } + + /// Nothing to do: every edit is written and synced when it is made, and + /// a file opened with 'w' is written on `close()`. + fn flush(&self) {} + + /// Deleting objects is not supported (h5py's `del f[name]`). + fn __delitem__(&self, key: &str) -> PyResult<()> { + Err(PyNotImplementedError::new_err(format!( + "cannot delete '{key}': deleting objects is not supported by clawhdf5" + ))) + } + /// The path (or URL) the file was opened with. #[getter] fn filename(&self) -> &str { @@ -389,6 +428,13 @@ impl PyFile { fn write_state_mut(&mut self) -> PyResult<&mut WriteState> { match &mut self.inner { Some(FileInner::Write(s)) => Ok(s), + Some(FileInner::Read(root)) if root.handle.is_writable() => { + Err(PyNotImplementedError::new_err( + "creating datasets or groups in an existing file is not supported by \ + clawhdf5's in-place editor (mode 'r+' changes values, shapes and \ + attributes)", + )) + } Some(FileInner::Read(_)) => Err(PyErr::new::( "cannot write to a file opened for reading", )), diff --git a/crates/clawhdf5-py/src/group.rs b/crates/clawhdf5-py/src/group.rs index 36506b9..4020f16 100644 --- a/crates/clawhdf5-py/src/group.rs +++ b/crates/clawhdf5-py/src/group.rs @@ -3,7 +3,7 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex, OnceLock}; -use pyo3::exceptions::{PyIOError, PyKeyError, PyOSError, PyValueError}; +use pyo3::exceptions::{PyIOError, PyKeyError, PyNotImplementedError, PyOSError, PyValueError}; use pyo3::prelude::*; use pyo3::types::PyList; @@ -301,12 +301,23 @@ impl PyGroup { state.lock().unwrap().datasets.push(spec); Ok(()) } - GroupInner::Read { .. } => Err(PyIOError::new_err( + GroupInner::Read(g) if g.handle.is_writable() => Err(PyNotImplementedError::new_err( + "creating datasets or groups in an existing file is not supported by \ + clawhdf5's in-place editor (mode 'r+' changes values, shapes and attributes)", + )), + GroupInner::Read(_) => Err(PyIOError::new_err( "cannot create datasets on a read-only group", )), } } + /// Deleting objects is not supported (h5py's `del group[name]`). + fn __delitem__(&self, key: &str) -> PyResult<()> { + Err(PyNotImplementedError::new_err(format!( + "cannot delete '{key}': deleting objects is not supported by clawhdf5" + ))) + } + /// Attribute access. #[getter] fn attrs(&self, py: Python<'_>) -> PyResult { diff --git a/crates/clawhdf5-py/src/handle.rs b/crates/clawhdf5-py/src/handle.rs index 8649f74..754de6f 100644 --- a/crates/clawhdf5-py/src/handle.rs +++ b/crates/clawhdf5-py/src/handle.rs @@ -2,25 +2,34 @@ //! //! Every read goes through [`Handle::with`], which releases the GIL and //! parses through `File::storage()`, so the same code serves a local file -//! (memory-mapped) and a remote one (`clawhdf5-remote`: range requests -//! through a block cache, so a network read never holds the GIL). +//! (memory-mapped), a remote one (`clawhdf5-remote`: range requests through +//! a block cache, so a network read never holds the GIL) and a file open +//! for editing. +//! +//! A file opened with `'r+'` also holds a [`FileEditor`]. An edit takes the +//! file's write lock, so no read runs while the file changes underneath it, +//! and reopens the file afterwards: reads after an edit see the new bytes +//! (a grown file, a new dataspace), never a stale mapping or chunk cache. +//! Objects that cache something an edit can change compare +//! [`Handle::generation`] with the value they cached it at. //! //! Lock discipline (no deadlock with the GIL): the file lock is only taken //! with the GIL released, and code that holds it never touches Python. use std::path::PathBuf; -use std::sync::{Arc, PoisonError, RwLock}; +use std::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex, PoisonError, RwLock}; -use clawhdf5_rs::File; +use clawhdf5_rs::{File, FileEditor}; use pyo3::exceptions::PyOSError; use pyo3::prelude::*; -use crate::to_py_err; +use crate::{panic_text, to_py_err}; /// Where the file's bytes come from. pub(crate) enum Source { /// A local path (memory-mapped). - Local(#[allow(dead_code)] PathBuf), + Local(PathBuf), /// A URL, read through `clawhdf5-remote`'s block cache. Remote { url: String, @@ -29,8 +38,14 @@ pub(crate) enum Source { } pub(crate) struct Handle { + /// The file as last opened; `None` if reopening it after an edit failed + /// (every read is then an error rather than a read of stale bytes). file: RwLock>, + /// For `'r+'`: the editor, until the file is closed. + editor: Option>>, source: Source, + /// Bumped by every edit. + generation: AtomicU64, pub offset_size: u8, pub length_size: u8, pub root: u64, @@ -41,13 +56,15 @@ fn closed_after_failed_reopen() -> PyErr { } impl Handle { - fn new(file: File, source: Source) -> Arc { + fn new(file: File, source: Source, editor: Option) -> Arc { let sb = file.superblock(); let (offset_size, length_size, root) = (sb.offset_size, sb.length_size, sb.root_group_address); Arc::new(Self { file: RwLock::new(Some(file)), + editor: editor.map(|e| Mutex::new(Some(e))), source, + generation: AtomicU64::new(0), offset_size, length_size, root, @@ -57,7 +74,25 @@ impl Handle { /// A local file, read-only. pub(crate) fn open_local(py: Python<'_>, path: &str) -> PyResult> { let file = py.detach(|| crate::no_panic(|| File::open(path).map_err(to_py_err)))?; - Ok(Self::new(file, Source::Local(PathBuf::from(path)))) + Ok(Self::new(file, Source::Local(PathBuf::from(path)), None)) + } + + /// A local file, open for in-place editing (`'r+'`): the editor takes + /// the file's exclusive lock and checks that it can edit the file, then + /// the file is opened for reading. + pub(crate) fn open_editable(py: Python<'_>, path: &str) -> PyResult> { + let (file, editor) = py.detach(|| { + crate::no_panic(|| { + let editor = FileEditor::open(path).map_err(to_py_err)?; + let file = File::open(path).map_err(to_py_err)?; + Ok((file, editor)) + }) + })?; + Ok(Self::new( + file, + Source::Local(PathBuf::from(path)), + Some(editor), + )) } /// A remote file (`http(s)://`, `s3://`, ...). @@ -79,6 +114,7 @@ impl Handle { url: url.to_string(), storage, }, + None, )) } @@ -102,6 +138,17 @@ impl Handle { }) } + /// Edits so far: objects that cache something an edit can change (a + /// dataset's shape, an object's attributes) re-read it when this moved. + pub(crate) fn generation(&self) -> u64 { + self.generation.load(Ordering::Acquire) + } + + /// Whether the file was opened for editing (`'r+'`), even if closed since. + pub(crate) fn is_writable(&self) -> bool { + self.editor.is_some() + } + /// The remote file's block cache. pub(crate) fn remote_storage(&self) -> Option<&clawhdf5_remote::RemoteStorage> { match &self.source { @@ -117,6 +164,61 @@ impl Handle { Source::Local(_) => None, } } + + /// Release the editor, and with it the file's lock. Objects still + /// open keep reading the file as it was last written; an edit through + /// them is an error. + pub(crate) fn close(&self) { + if let Some(ed) = &self.editor { + ed.lock().unwrap_or_else(PoisonError::into_inner).take(); + } + } + + /// Apply one edit with the GIL released. No read runs while it writes, + /// and the file is reopened afterwards — also after a failed edit, since + /// a commit that failed part-way may have changed the file. + pub(crate) fn edit( + &self, + py: Python<'_>, + f: impl FnOnce(&mut FileEditor) -> Result + Send, + ) -> PyResult { + let Some(editor) = &self.editor else { + return Err(PyOSError::new_err(match self.source { + Source::Remote { .. } => "remote files are read-only", + Source::Local(_) => { + "the file is open read-only; open it with mode 'r+' to change it" + } + })); + }; + let Source::Local(path) = &self.source else { + return Err(PyOSError::new_err("remote files are read-only")); + }; + py.detach(|| { + let mut ed = editor.lock().unwrap_or_else(PoisonError::into_inner); + let ed = ed + .as_mut() + .ok_or_else(|| PyOSError::new_err("the file is closed"))?; + let mut file = self.file.write().unwrap_or_else(PoisonError::into_inner); + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| f(ed))); + // Drop the old mapping and its chunk cache before reopening. + *file = None; + let reopened = std::panic::catch_unwind(|| File::open(path)); + self.generation.fetch_add(1, Ordering::AcqRel); + match reopened { + Ok(Ok(f)) => *file = Some(f), + Ok(Err(e)) => return Err(to_py_err(e)), + Err(_) => return Err(closed_after_failed_reopen()), + } + drop(file); + match result { + Ok(r) => r.map_err(to_py_err), + Err(p) => Err(crate::InternalError::new_err(format!( + "clawhdf5 internal error (please report it): {}", + panic_text(&*p) + ))), + } + }) + } } /// A `clawhdf5_remote::Error` as a Python exception: the network side diff --git a/crates/clawhdf5-py/src/lib.rs b/crates/clawhdf5-py/src/lib.rs index db89d8d..4ae81f7 100644 --- a/crates/clawhdf5-py/src/lib.rs +++ b/crates/clawhdf5-py/src/lib.rs @@ -7,11 +7,19 @@ //! //! with clawhdf5.File('data.h5', 'r') as f: //! data = f['dataset_name'][:] +//! +//! with clawhdf5.File('http://host/data.h5') as f: # range requests +//! block = f['dataset_name'][10:20] +//! +//! with clawhdf5.File('data.h5', 'r+') as f: # in-place edits +//! f['dataset_name'][0] = 1.5 +//! f.attrs['note'] = 'edited' //! ``` mod attrs; mod convert; mod dataset; +mod edit; mod file; mod group; mod handle; diff --git a/crates/clawhdf5-py/tests/test_edit.py b/crates/clawhdf5-py/tests/test_edit.py new file mode 100644 index 0000000..99090e2 --- /dev/null +++ b/crates/clawhdf5-py/tests/test_edit.py @@ -0,0 +1,742 @@ +"""In-place editing: clawhdf5.File(path, 'r+') against h5py. + +Every edit is applied twice, to two copies of the same file: once through +h5py (libhdf5) and once through clawhdf5 (FileEditor). After every edit both +files are read back with h5py and must hold the same shapes, values and +attributes; clawhdf5's own view must agree; when h5py refuses an edit, +clawhdf5 must refuse it too and leave its file as it was. Files are written +by h5py (libver earliest and latest, so every chunk index kind) and by +clawhdf5; `h5dump` must read every result.""" + +import io +import os +import shutil +import subprocess +import threading + +import numpy as np +import pytest + +import clawhdf5 + + +# --------------------------------------------------------------------------- +# Files +# --------------------------------------------------------------------------- + +ENUM = {"RED": 0, "GREEN": 1, "BLUE": 7} + + +def _h5py_file(h5py, path, libver): + rng = np.random.default_rng(1) + with h5py.File(path, "w", libver=libver) as f: + f.create_dataset("i4", data=np.arange(60, dtype="u2", "i8", "f8"] + + +def _quiet(f): + with np.errstate(all="ignore"): + return f() + + +def _libhdf5_undefined(vals, target): + """The values whose conversion to `target` libhdf5 2.0 (h5py 3.16) gets + wrong even in native byte order, where its C casts are undefined + behaviour; clawhdf5 saturates them as libhdf5's range handling + intends (docs/known-issues.md): + + - half floats into unsigned integers: negatives wrap (-1 -> 65535) and + +inf becomes 0; into signed integers, +-inf becomes the minimum; + - a float equal to the integer maximum rounded up in the float's + precision (float32(2**31 - 1) == 2**31 -> int32, float64(2**64 - 1) + -> uint64) becomes the minimum (or 0); + - a double between 65504 and 65520 into a half float becomes infinity + (IEEE rounds it down to 65504, as numpy does). + """ + bad = np.zeros(vals.shape, dtype=bool) + if vals.dtype.kind != "f": + return bad + if target.kind in "iu": + if vals.dtype.itemsize == 2: + bad |= np.isinf(vals) + if target.kind == "u": + bad |= vals <= -1 + top = _quiet(lambda: np.array(np.iinfo(target).max).astype(vals.dtype)) + if float(top) > np.iinfo(target).max: + bad |= vals == top + if target.kind == "f" and target.itemsize == 2 and vals.dtype.itemsize > 2: + bad |= (np.abs(vals) > 65504) & (np.abs(vals) < 65520) + return bad + + +def test_numeric_conversions_match_h5py(h5py, tmp_path): + """Every numeric source dtype into every numeric dataset dtype, with + values at and beyond the targets' limits, as libhdf5 converts them.""" + edge = np.array([0, 1, -1, -0.3, 2.5, -2.5, 3.7, -3.7, 127.9, -128.9, 200.5, 255.5, 256, -129, + 32767.5, 40000, 65504, 70000, -70000, 2**31 - 1, 2**31, -2**31 - 1, + 4e9, 1e15, -1e15, 1e19, 1e300, -1e300, np.inf, -np.inf]) + sources = { + "f8": edge, + "f4": _quiet(lambda: edge.astype(" {target}" + try: + with h5py.File(path_t, "r+") as f: + f["d"][...] = _native_conversion(h5py, vals, f["d"].dtype) + except Exception: # noqa: BLE001 + with clawhdf5.File(path_o, "r+") as f, pytest.raises(Exception): + f["d"][...] = vals + continue + with clawhdf5.File(path_o, "r+") as f: + f["d"][...] = vals + with h5py.File(path_t, "r") as a, h5py.File(path_o, "r") as b: + assert a["d"][...].tobytes() == b["d"][...].tobytes(), ( + f"{what}: h5py {a['d'][...].tolist()} clawhdf5 {b['d'][...].tolist()}" + ) + + +def test_nan_into_an_integer_dataset_is_refused(h5py, tmp_path): + """libhdf5 stores NaN as an arbitrary integer (0, the minimum or 2**63, + depending on the type); clawhdf5 refuses and writes nothing.""" + path = str(tmp_path / "nan.h5") + with h5py.File(path, "w") as f: + f.create_dataset("d", data=np.arange(4, dtype="f4": np.array([1.5, 2.5], dtype=">f4"), + "f2": np.float16(0.5), + "b": True, + "barr": np.array([True, False]), + "c16": np.complex128(1 + 2j), + "bytes": b"raw", + "sarr": np.array([b"a", b"bcd"]), + "str": "héllo", + "strs": ["x", "yz"], + "2d": np.arange(12, dtype="f4"].dtype == np.dtype(">f4") + assert a["f2"].dtype == np.float16 + assert a["b"] is np.True_ or a["b"] == True # noqa: E712 + assert a["barr"].dtype == np.bool_ + assert a["c16"] == 1 + 2j + assert a["bytes"] == b"raw" + assert list(a["sarr"]) == [b"a", b"bcd"] + # str is stored as fixed-length UTF-8: h5py reads bytes. + assert a["str"].decode("utf-8") == "héllo" + assert [x.decode() for x in a["strs"]] == ["x", "yz"] + assert a["2d"].shape == (3, 4) and a["2d"].dtype == np.dtype("/dev/null 2>&1 && "$PYTHON" -c "import pytest" >/dev/null 2>&1; then