diff --git a/CHANGELOG.md b/CHANGELOG.md index 2ee4cca..bd4c1c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,148 @@ Design: `docs/design/swmr.md`. `HttpStorage` pins the length), `MmapFile`/`LazyFile`, and refreshing groups or attributes (a SWMR writer cannot add objects or attributes). +### Correctness: edits planned from another file after a rename or `chdir` (2026-09-27) +- **`FileEditor` planned each edit by re-opening its path but wrote + through the file it held open** (fixed 2026-09-27; on main since PR #18, + no release). When the path came to name another file between edits — a + rename or replacement, or, for a relative path, a change of working + directory — an edit was laid out from the other file's metadata and + written into the held one, corrupting it (h5py: "invalid dataset size, + likely file corruption"). The Python `'r+'` handle had the same flaw in + its reads: it reopened the path after every edit, so reads came from the + other file. The editor now plans every edit from the file it holds, and + its path is canonicalised at open. New `FileEditor::reader()` opens the + held file anew for reading (on Linux through `/proc/self/fd`, so it + follows a renamed file; elsewhere by the path, refused when the path no + longer names the held file), without sharing the editor's lock; the + Python handle reads through it, and a `'w'` file is written at the + absolute path it was opened with. Tests: `edit_tests.rs`'s + `edits_go_to_the_file_held_not_the_path`; `test_edit.py`'s + `test_relative_path_and_chdir` and `test_path_replaced_between_edits` + (the review's repro). + +### Correctness: zero extents in Fixed/Extensible Array chunk indexes (2026-09-27) +- **A chunked dataset whose maximum (or, with none recorded, current) + extent is 0 along a dimension made the reader divide by zero** (fixed + 2026-09-27): `h5rs check` panicked ("attempt to divide by zero", + `chunk_grid.rs`) and the next `FileEditor::resize` failed with an + internal error. The unfixed editor produced such files by resizing a + clawhdf5-written dataset to a zero extent (12 of 30 extra random-edit + seeds on clawhdf5-written files). Such an index has no slot for any + chunk of the dataset, and `ChunkGrid::offsets` now says so instead of + dividing by the zero stride. Tests: `chunk_grid`'s + `zero_extent_has_no_chunks`, `edit_interop.rs`'s + `zero_extent_resizes_without_a_recorded_maximum` (a file the unfixed + editor left checks clean and resizes on; a 2.7.0-written file through + zero extents checks clean at each step), and `test_edit.py`'s random + edits on seeds 10 to 39 of a clawhdf5-written file. + +### Correctness: resizing chunked datasets with no recorded maximum (2026-09-27) +- **`FileEditor::resize` scrambled the values of a chunked dataset whose + dataspace records no maximum dimensions when it shrank it** (fixed + 2026-09-27). The editor shipped on main in PR #18 (a4c2ace) and reached + Python as `Dataset.resize` in `'r+'` files; no release has it. clawhdf5's + writer stores such a dataspace for every chunked dataset created without + a `maxshape`, with a Fixed Array (or Single Chunk) chunk index. The + editor patched only the current dimensions, and with no maximum + recorded the maximum is the current dimensions — which is also what the + Fixed Array linearises chunks by — so a shrink moved every chunk after + the first row: h5py, h5dump and our reader all read wrong values + without complaint (20x20, chunks 6x6, resized to 15x15: row 6 read + `0 0 0 0 0 0 120 ...`). After a shrink the dataset could not grow back + either (`3 exceeds the maximum 0`). libhdf5 itself never writes such a + dataspace (`H5S_set_extent_simple` records the maximum, equal to the + dimensions when none is given); reading one, `H5S_extent_get_dims` + reports the current dimensions as the maximum and `H5S_set_extent` + checks against no maximum at all, so libhdf5's own `H5Dset_extent` + scrambles such a file the same way (and lets it grow past its Fixed + Array). The editor now records the maximum libhdf5 would have written + — the dimensions before the first resize, the ones the index was built + with — then changes the current ones (the dataspace message grows by one + length per dimension and moves in the header when it has to). The + dataset then shrinks, grows back to that extent and refuses more, as + one libhdf5 wrote would. The writer (`FileBuilder`) now records the + maximum of every chunked dataset too, as libhdf5 does, so h5py can + resize what it writes (8 more bytes per dimension). Tests: + `crates/clawhdf5/tests/edit_resize_interop.rs` (a 2.7.0-written fixture, + new `FileBuilder` files and h5py files through shrinks, zero extents + and growth, checked against a model with our reader and h5py; h5py + resizing a `FileBuilder` file) and `test_edit.py`'s numpy-model checks. + +### 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`. +- Boolean-mask keys (`ds[mask]`, `ds[mask] = v`), which h5py supports, + raise `NotImplementedError` (they raised `TypeError`). +- 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`, + `azure` features) through `clawhdf5-remote`'s `open_url`: range requests + through the block cache, the whole read API (groups, attributes, every + dataset type and index the local reader handles). A URL is any + `scheme://…`; a remote file is read-only (another mode is a + `ValueError`). **`clawhdf5.File.open_url(url, **options)`** takes + `block_size`, `cache_size`, `headers`, `retries`, `timeout`, + `allow_full_download`, `max_full_download`, `require_validator`, + `max_redirects` and `max_parallel`; `File.remote_stats` gives the block + cache's counters. The default wheel builds plain HTTP only (no C: rustls + needs ring, and the cloud clients aws-lc-rs), and `ci-test.sh`'s no-C + check now covers `clawhdf5-py`. +- **Every read parses through `File::storage()`** instead of + `File::as_bytes()` (path lookups, object headers, dataspaces, attributes, + group listings, variable-length data through the global heap), inside one + shared file handle that releases the GIL for all file access, not only + dataset reads: a read waiting on the network lets other Python threads + run. A failed read of the storage (a network error, a file changed on + the server) is an `OSError`, never a `KeyError`/`ValueError` and never + data; `key in group` raises it instead of answering `False`. +- Tests: the whole read-vs-h5py suite also runs over HTTP (default 1 MiB + blocks and 1 KiB blocks) against a range-capable `http.server` in the + test process, plus `tests/test_remote.py`: request counts of a small + read, cache hits, a server without `Range` support (refused, or a + whole download when allowed), a file changed on the server, a server + that hangs up, 16 threads on one remote file, and a thread that keeps + running while a read waits on 0.2 s requests. + ### Range reads, milestone M3: remote files (2026-09-26) - **New crate `clawhdf5-remote`.** `open_url("http://host/file.h5")` gives a `clawhdf5::File` (through `File::open_storage`) that reads the file by diff --git a/README.md b/README.md index 2b9c1a1..70ced48 100644 --- a/README.md +++ b/README.md @@ -574,8 +574,36 @@ with clawhdf5.File("data.h5", "r") as f: records = f["table"] # compound -> numpy structured array ids = records["id"] # one field + +# A file on a web server: range requests through a block cache, nothing +# downloaded up front; the same read API. The GIL is released while waiting. +with clawhdf5.File("http://data.example.org/run42.h5") as f: + first = f["group/temperatures"][0] +f = clawhdf5.File.open_url("http://data.example.org/run42.h5", block_size=256 * 1024, + 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 +object-store URLs. + Reads cover integers and IEEE floats of every width in either byte order, `bool`, enums, complex, fixed and variable-length strings, variable-length sequences, opaque, HDF5 array types and compounds; other types (references, @@ -590,7 +618,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-format/src/chunk_grid.rs b/crates/clawhdf5-format/src/chunk_grid.rs index 9e03b9d..8957512 100644 --- a/crates/clawhdf5-format/src/chunk_grid.rs +++ b/crates/clawhdf5-format/src/chunk_grid.rs @@ -135,6 +135,12 @@ impl ChunkGrid { let mut rem = index; for p in 0..rank { let d = self.order[p]; + // A zero stride: a later dimension has no chunks (its maximum, + // or with none recorded its current extent, is 0), so no slot of + // the index is a chunk of the dataset. + if self.down[p] == 0 { + return None; + } let scaled = rem / self.down[p]; rem %= self.down[p]; if scaled >= self.cur_chunks[d] { @@ -193,6 +199,28 @@ mod tests { assert_eq!(g.offsets(11), Some(vec![2, 3])); } + #[test] + fn zero_extent_has_no_chunks() { + // No maximum recorded and a zero current dimension: every stride + // before it is 0 (this divided by zero). + let g = ChunkGrid::fixed_array(&[1, 0], None, &[6, 6]).unwrap(); + for i in 0..16 { + assert_eq!(g.offsets(i), None); + } + let g = ChunkGrid::fixed_array(&[0, 0, 3], Some(&[4, 0, 3]), &[2, 2, 3]).unwrap(); + for i in 0..16 { + assert_eq!(g.offsets(i), None); + } + let g = ChunkGrid::extensible_array(&[0, 5], Some(&[u64::MAX, 0]), &[2, 2]).unwrap(); + for i in 0..16 { + assert_eq!(g.offsets(i), None); + } + // A zero last dimension leaves the other strides alone. + let g = ChunkGrid::fixed_array(&[4, 0], Some(&[4, 6]), &[2, 3]).unwrap(); + assert_eq!(g.offsets(0), None); + assert_eq!(g.linear_index(&[1, 1]), 3); + } + #[test] fn rejects_two_unlimited_dims_after_the_first() { assert!(ChunkGrid::fixed_array(&[4, 6], Some(&[u64::MAX, u64::MAX]), &[2, 3]).is_err()); diff --git a/crates/clawhdf5-format/src/file_writer.rs b/crates/clawhdf5-format/src/file_writer.rs index 1a1af83..c64e1cf 100644 --- a/crates/clawhdf5-format/src/file_writer.rs +++ b/crates/clawhdf5-format/src/file_writer.rs @@ -130,6 +130,22 @@ pub(crate) fn build_chunked_dataset_oh( fill_message: &[u8], refcount: u32, ) -> Result, FormatError> { + // libhdf5 records every simple dataspace's maximum dimensions (the + // dimensions themselves when none are given, `H5S_set_extent_simple`). + // Without them libhdf5 takes the maximum to be the current dimensions, + // so a resize by libhdf5 (h5py's `Dataset.resize`) would also change + // the maximum a Fixed Array chunk index is laid out by, and move every + // chunk already written. + let recorded; + let ds = if ds.space_type == DataspaceType::Simple && ds.max_dimensions.is_none() { + recorded = Dataspace { + max_dimensions: Some(ds.dimensions.clone()), + ..ds.clone() + }; + &recorded + } else { + ds + }; let mut w = ObjectHeaderWriter::new(); w.add_message_with_flags(MessageType::Datatype, dt.serialize(), 0x01); w.add_message(MessageType::Dataspace, ds.serialize(LENGTH_SIZE)); diff --git a/crates/clawhdf5-io/src/mmap.rs b/crates/clawhdf5-io/src/mmap.rs index 756d5e3..113693a 100644 --- a/crates/clawhdf5-io/src/mmap.rs +++ b/crates/clawhdf5-io/src/mmap.rs @@ -39,6 +39,23 @@ impl MmapReader { Ok(Self { _file: file, mmap }) } + /// Memory-map a file that is already open (for reading). + /// + /// The mapping references `file`'s open file description for as long as + /// it lives, so a `flock` taken through that description (or a + /// `try_clone` of it) is held until the reader is dropped. + /// + /// # Safety + /// + /// The same contract as [`open`](Self::open): the file must not be + /// modified while the mapping is active. + pub fn from_file(file: fs::File) -> io::Result { + // SAFETY: a read-only mapping; the caller keeps the file unmodified + // while it is alive. + let mmap = unsafe { Mmap::map(&file)? }; + Ok(Self { _file: file, mmap }) + } + /// Zero-copy access to the entire file contents. pub fn as_bytes(&self) -> &[u8] { &self.mmap diff --git a/crates/clawhdf5-py/Cargo.toml b/crates/clawhdf5-py/Cargo.toml index 3513a62..8246440 100644 --- a/crates/clawhdf5-py/Cargo.toml +++ b/crates/clawhdf5-py/Cargo.toml @@ -17,11 +17,20 @@ crate-type = ["cdylib", "rlib"] [dependencies] clawhdf5_rs = { path = "../clawhdf5", version = "2.7.0", package = "clawhdf5" } clawhdf5-format = { path = "../clawhdf5-format", version = "2.7.0" } +# Remote files (`clawhdf5.File(url)`): plain HTTP by default, which builds +# no C. HTTPS and the object stores are opt-in features below. +clawhdf5-remote = { path = "../clawhdf5-remote", version = "2.7.0" } pyo3 = "0.29" numpy = "0.29" [features] extension-module = ["pyo3/extension-module"] +# https:// URLs (rustls with ring, which compiles C and assembly). +https = ["clawhdf5-remote/https"] +# s3://, gs://, az:// URLs (object_store; its cloud clients build aws-lc-rs, C). +s3 = ["clawhdf5-remote/s3"] +gcs = ["clawhdf5-remote/gcs"] +azure = ["clawhdf5-remote/azure"] [package.metadata.docs.rs] features = [] diff --git a/crates/clawhdf5-py/README.md b/crates/clawhdf5-py/README.md index 95cae12..06eebec 100644 --- a/crates/clawhdf5-py/README.md +++ b/crates/clawhdf5-py/README.md @@ -43,8 +43,9 @@ with clawhdf5.File("data.h5", "r") as f: Other types raise `TypeError`. - Keys are h5py's: integers, slices with a positive step, `...`, one increasing list of integers, compound field names. Each maps onto a - hyperslab selection. `None`, negative steps and boolean masks are refused - with h5py's errors. + hyperslab selection. `None` and negative steps are refused + with h5py's errors; boolean masks (which h5py supports) raise + `NotImplementedError`, for reads and writes. - What is read from the file: a selection whose bounding box covers at most half the dataset decodes only the chunks (or contiguous rows) the box overlaps. The library decodes the whole dataset for a larger box @@ -61,6 +62,36 @@ with clawhdf5.File("data.h5", "r") as f: - Attributes return what h5py returns; `clawhdf5.Empty` stands for a null dataspace (h5py's `Empty`). +## Remote files + +A URL instead of a path reads the file where it is, through +`clawhdf5-remote`: HTTP range requests through a block cache (1 MiB blocks, +64 MiB budget by default), fetching only the blocks a read needs. The whole +read API works the same, and the GIL is released while waiting on the +network. + +```python +f = clawhdf5.File("http://host/data.h5") # default options +f = clawhdf5.File.open_url( + "http://host/data.h5", + block_size=256 * 1024, cache_size=128 << 20, # the block cache + headers={"Authorization": "Bearer ..."}, # sent to this origin only + retries=3, timeout=30.0, max_redirects=5, max_parallel=8, + allow_full_download=False, # a server without Range support: refuse + require_validator=False, # refuse servers without ETag/Last-Modified +) +f.remote_stats # {'requests': ..., 'bytes_fetched': ..., 'hits': ..., ...} +``` + +- The file is pinned when opened (ETag or Last-Modified, and length): if it + changes on the server, reads raise `OSError` instead of mixing versions. + Network failures are `OSError` too. +- Remote files are read-only. +- Schemes: the default build (no C) reads `http://`. `https://` needs + `maturin develop --release --features https` (rustls with ring, which + compiles C); `s3://`, `gs://` and `az://` need the `s3`, `gcs` and + `azure` features (credentials from the environment; aws-lc-rs, C). + ## Writing `clawhdf5.File(path, "w")` with `create_dataset(name, data=array, @@ -68,6 +99,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 @@ -76,7 +142,12 @@ pytest crates/clawhdf5-py/tests ``` `tests/test_read_vs_h5py.py` compares every read with h5py on a file h5py -writes. `scripts/ci-test.sh` builds the wheel and runs these in CI. +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); `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 fd3f0b3..5b3f365 100644 --- a/crates/clawhdf5-py/src/attrs.rs +++ b/crates/clawhdf5-py/src/attrs.rs @@ -1,22 +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::{OwnedAttrValue, PyEmpty, attr_value_to_py, node, py_to_attr_value}; +use crate::handle::Handle; +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 { - file: Arc, - attrs: Vec, - }, + Read(ReadAttrs), /// Writable attribute list shared with a parent (PyFile or PyGroup). Write(Arc>>), } @@ -26,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, @@ -36,10 +79,21 @@ pub struct PyAttrs { impl PyAttrs { /// The attributes of the object at `addr` (whose path is `path`) in a /// file opened for reading. - pub(crate) fn read(file: Arc, addr: u64, path: &str) -> PyResult { - let attrs = node::attributes(&file, addr, path)?; + pub(crate) fn read( + py: Python<'_>, + handle: Arc, + addr: u64, + path: &str, + ) -> PyResult { + let generation = handle.generation(); + let attrs = Arc::new(handle.with(py, |f| node::attributes(f, addr, path))?); Ok(Self { - inner: AttrsInner::Read { file, attrs }, + inner: AttrsInner::Read(ReadAttrs { + handle, + addr, + path: path.to_string(), + cache: Mutex::new((generation, attrs)), + }), }) } @@ -49,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 { file, attrs } => match attrs.iter().find(|a| a.name == key) { - Some(attr) => Ok(attr_to_py(py, file, 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}')" ))), @@ -74,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)), } } @@ -113,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())) @@ -131,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() @@ -146,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 { file, attrs } => attrs + AttrsInner::Read(r) => r + .current(py)? .iter() - .map(|a| attr_to_py(py, file, a).map(Bound::unbind)) + .map(|a| attr_to_py(py, &r.handle, a).map(Bound::unbind)) .collect::>()?, AttrsInner::Write(store) => store .lock() @@ -167,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 { file, attrs } => attrs + AttrsInner::Read(r) => r + .current(py)? .iter() - .map(|a| Ok((a.name.clone(), attr_to_py(py, file, a)?.unbind()))) + .map(|a| Ok((a.name.clone(), attr_to_py(py, &r.handle, a)?.unbind()))) .collect::>()?, AttrsInner::Write(store) => store .lock() @@ -189,12 +308,11 @@ impl PyAttrs { /// An attribute's value as h5py returns it. fn attr_to_py<'py>( py: Python<'py>, - file: &clawhdf5_rs::File, + handle: &Handle, attr: &AttributeMessage, ) -> PyResult> { crate::no_panic(|| { - let sb = file.superblock(); - let conv = Converter::new(py, &attr.datatype, sb.offset_size) + let conv = Converter::new(py, &attr.datatype, handle.offset_size) .map_err(|e| prefix_err(py, &attr.name, e))?; if node::is_null(&attr.dataspace) { return Ok(PyEmpty::new(conv.dtype).into_pyobject(py)?.into_any()); @@ -216,12 +334,11 @@ fn attr_to_py<'py>( ))); } let raw = &attr.raw_data[..want]; - let file_data = file.as_bytes(); - let (osz, lsz, unit) = (sb.offset_size, sb.length_size, conv.vl_unit); - Elements::Vl( - py.detach(|| resolve_vl(file_data, raw, n, osz, lsz, unit)) - .map_err(|e| PyValueError::new_err(format!("attribute {}: {e}", attr.name)))?, - ) + let (osz, lsz, unit) = (handle.offset_size, handle.length_size, conv.vl_unit); + let what = format!("attribute {}", attr.name); + Elements::Vl(handle.with(py, |f| { + resolve_vl(f.storage(), raw, n, osz, lsz, unit).map_err(|e| e.into_py(&what)) + })?) } else { Elements::Bytes(attr.raw_data.clone()) }; @@ -244,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/convert.rs b/crates/clawhdf5-py/src/convert.rs index 4ca7546..c8c3d68 100644 --- a/crates/clawhdf5-py/src/convert.rs +++ b/crates/clawhdf5-py/src/convert.rs @@ -17,6 +17,7 @@ use std::collections::HashMap; use clawhdf5_format::datatype::{CharacterSet, Datatype, DatatypeByteOrder}; use clawhdf5_format::global_heap::GlobalHeapCollection; +use clawhdf5_format::storage::Storage; use numpy::PyArray1; use pyo3::exceptions::{PyTypeError, PyValueError}; use pyo3::prelude::*; @@ -538,16 +539,16 @@ fn object_array<'py>( /// their bytes: each element's stored length times `unit` (1 for strings, /// the base type's size for sequences). Pure Rust, so it runs without the /// GIL. -pub(crate) fn resolve_vl( - file_data: &[u8], +pub(crate) fn resolve_vl( + file: &S, raw: &[u8], count: usize, offset_size: u8, length_size: u8, unit: usize, -) -> Result>, String> { +) -> Result>, VlError> { let refs = clawhdf5_format::vl_data::parse_vl_references(raw, count as u64, offset_size) - .map_err(|e| e.to_string())?; + .map_err(|e| VlError::Invalid(e.to_string()))?; let undefined = match offset_size { 2 => 0xFFFF, 4 => 0xFFFF_FFFF, @@ -558,43 +559,70 @@ pub(crate) fn resolve_vl( for vl in &refs { if vl.collection_address == 0 || vl.collection_address == undefined { if vl.length != 0 { - return Err(format!( + return Err(VlError::Invalid(format!( "variable-length element of length {} has no heap address", vl.length - )); + ))); } out.push(Vec::new()); continue; } let coll = match collections.entry(vl.collection_address) { std::collections::hash_map::Entry::Occupied(e) => e.into_mut(), - std::collections::hash_map::Entry::Vacant(e) => { - let addr = usize::try_from(vl.collection_address) - .map_err(|_| "global heap address out of range".to_string())?; - e.insert( - GlobalHeapCollection::parse(file_data, addr, length_size) - .map_err(|e| e.to_string())?, - ) - } + std::collections::hash_map::Entry::Vacant(e) => e.insert( + GlobalHeapCollection::parse_in(file, vl.collection_address, length_size) + .map_err(VlError::from_format)?, + ), }; - let index = u16::try_from(vl.object_index) - .map_err(|_| format!("global heap object index {} out of range", vl.object_index))?; + let index = u16::try_from(vl.object_index).map_err(|_| { + VlError::Invalid(format!( + "global heap object index {} out of range", + vl.object_index + )) + })?; let obj = coll.get_object(index).ok_or_else(|| { - format!( + VlError::Invalid(format!( "global heap object {index} not found in the collection at {}", vl.collection_address - ) + )) })?; let need = (vl.length as usize) .checked_mul(unit) - .ok_or("variable-length element too long")?; + .ok_or_else(|| VlError::Invalid("variable-length element too long".into()))?; if need > obj.data.len() { - return Err(format!( + return Err(VlError::Invalid(format!( "variable-length element of {need} bytes in a {}-byte heap object", obj.data.len() - )); + ))); } out.push(obj.data[..need].to_vec()); } Ok(out) } + +/// Why variable-length elements could not be resolved. +#[derive(Debug)] +pub(crate) enum VlError { + /// Reading the file failed (a network error on a remote file). + Storage(String), + /// The references or the heap are not valid. + Invalid(String), +} + +impl VlError { + fn from_format(e: clawhdf5_format::error::FormatError) -> Self { + match e { + clawhdf5_format::error::FormatError::Storage(_) => VlError::Storage(e.to_string()), + e => VlError::Invalid(e.to_string()), + } + } + + /// As a Python exception, the message prefixed with `what`: a storage + /// failure is an `OSError`, anything else a `ValueError`. + pub(crate) fn into_py(self, what: &str) -> PyErr { + match self { + VlError::Storage(m) => pyo3::exceptions::PyOSError::new_err(format!("{what}: {m}")), + VlError::Invalid(m) => PyValueError::new_err(format!("{what}: {m}")), + } + } +} diff --git a/crates/clawhdf5-py/src/dataset.rs b/crates/clawhdf5-py/src/dataset.rs index bcfd852..d837f73 100644 --- a/crates/clawhdf5-py/src/dataset.rs +++ b/crates/clawhdf5-py/src/dataset.rs @@ -6,20 +6,52 @@ //! whole dataset instead); the //! bytes it returns become the numpy array's buffer without a copy (see //! `convert`). All file access and decoding runs with the GIL released, so -//! Python threads reading the same or different datasets run in parallel. +//! 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 pyo3::exceptions::{PyTypeError, PyValueError}; +use clawhdf5_rs::File; +use pyo3::exceptions::{PyNotImplementedError, PyOSError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyList, PyTuple}; use crate::attrs::PyAttrs; -use crate::convert::{Converter, Elements, resolve_vl}; +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 { + /// `None` for a dataset with a null dataspace (h5py's `Empty`). + shape: Option>, + chunks: Option>, + datatype: Datatype, +} + +impl DatasetMeta { + pub(crate) fn load(f: &File, addr: u64, hdr: &ObjectHeader, path: &str) -> PyResult { + let null = node::is_null(&node::dataspace(f, hdr, path)?); + let ds = f.dataset_at(addr).map_err(to_py_err)?; + let shape = if null { + None + } else { + Some(ds.shape().map_err(to_py_err)?) + }; + let datatype = ds.raw_datatype().map_err(to_py_err)?; + let chunks = shape + .as_ref() + .and_then(|s| node::chunk_shape(f, hdr, s.len())); + Ok(Self { + shape, + chunks, + datatype, + }) + } +} /// A dataset in a file opened for reading. /// @@ -30,13 +62,14 @@ use crate::{PyEmpty, node, to_py_err}; /// ``` #[pyclass(name = "Dataset")] pub struct PyDataset { - file: Arc, + handle: Arc, path: String, /// 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, @@ -45,39 +78,25 @@ pub struct PyDataset { } impl PyDataset { - pub(crate) fn open( + pub(crate) fn new( py: Python<'_>, - file: Arc, + handle: Arc, path: String, addr: u64, - hdr: &ObjectHeader, - ) -> PyResult { - crate::no_panic(|| { - let null = node::is_null(&node::dataspace(&file, hdr)?); - let (shape, datatype) = { - let ds = file.dataset_at(addr).map_err(to_py_err)?; - let shape = if null { - None - } else { - Some(ds.shape().map_err(to_py_err)?) - }; - (shape, ds.raw_datatype().map_err(to_py_err)?) - }; - let conv = Converter::new(py, &datatype, file.superblock().offset_size) - .map_err(|e| e.value(py).to_string()); - let chunks = shape - .as_ref() - .and_then(|s| node::chunk_shape(&file, hdr, s.len())); - Ok(Self { - file, - path, - addr, - shape, - chunks, - datatype, - conv, - }) - }) + 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: Mutex::new((generation, meta.shape)), + chunks: meta.chunks, + datatype: meta.datatype, + conv, + } } fn converter(&self) -> PyResult<&Converter> { @@ -86,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() { @@ -103,10 +166,10 @@ impl PyDataset { }; let (reads, list_axis) = plan.reads(dims, chunk_len, elem_size); let read_shape = plan.read_shape(); - let file = &*self.file; + let handle = &*self.handle; let addr = self.addr; // Everything below touches only Rust data: release the GIL. - let read = || -> Result { + let read = |file: &File| -> Result { let ds = file.dataset_at(addr)?; let mut blocks = Vec::with_capacity(reads.len()); for read in reads { @@ -147,7 +210,7 @@ impl PyDataset { let sb = file.superblock(); let n = read_shape.iter().product(); resolve_vl( - file.as_bytes(), + file.storage(), &raw, n, sb.offset_size, @@ -155,12 +218,20 @@ impl PyDataset { unit, ) .map(Elements::Vl) - .map_err(ReadError::Other) + .map_err(ReadError::Vl) }; let data = py .detach(|| { - std::panic::catch_unwind(std::panic::AssertUnwindSafe(read)) - .unwrap_or_else(|p| Err(ReadError::Panic(crate::panic_text(&*p)))) + handle + .with_detached(|f| { + Ok( + std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| read(f))) + .unwrap_or_else(|p| { + Err(ReadError::Panic(crate::panic_text(&*p))) + }), + ) + }) + .unwrap_or_else(|e| Err(ReadError::Py(e))) }) .map_err(|e| e.into_py(&self.path))?; let joined = conv.to_array(py, data, &read_shape, false)?; @@ -183,6 +254,8 @@ impl PyDataset { /// An error from the read closure, turned into a Python error with the GIL. enum ReadError { Lib(clawhdf5_rs::Error), + Vl(VlError), + Py(PyErr), Other(String), Panic(String), } @@ -197,6 +270,8 @@ impl ReadError { fn into_py(self, path: &str) -> PyErr { match self { ReadError::Lib(e) => to_py_err(e), + ReadError::Vl(e) => e.into_py(&node::name(path)), + ReadError::Py(e) => e, ReadError::Other(msg) => PyValueError::new_err(format!("{}: {msg}", node::name(path))), ReadError::Panic(msg) => crate::InternalError::new_err(format!( "{}: clawhdf5 internal error (please report it): {msg}", @@ -243,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)), } @@ -252,22 +327,32 @@ impl PyDataset { /// The maximum shape (`None` per unlimited dimension), like h5py. #[getter] fn maxshape<'py>(&self, py: Python<'py>) -> PyResult> { - crate::no_panic(|| { - let Some(shape) = &self.shape else { - return Ok(py.None().into_bound(py)); - }; - let max = self - .file - .dataset_at(self.addr) - .and_then(|ds| ds.max_dimensions()) - .map_err(to_py_err)? - .unwrap_or_else(|| shape.clone()); - let items: Vec> = max - .into_iter() - .map(|d| (d != u64::MAX).then_some(d)) - .collect(); - Ok(PyTuple::new(py, items)?.into_any()) - }) + let Some(shape) = self.dims(py)? else { + return Ok(py.None().into_bound(py)); + }; + let addr = self.addr; + let max = self + .handle + .with(py, |f| { + f.dataset_at(addr) + .and_then(|ds| ds.max_dimensions()) + .map_err(to_py_err) + })? + .unwrap_or(shape); + let items: Vec> = max + .into_iter() + .map(|d| (d != u64::MAX).then_some(d)) + .collect(); + 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. @@ -277,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`. @@ -293,10 +378,11 @@ 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) -> PyResult { - PyAttrs::read(Arc::clone(&self.file), self.addr, &self.path) + fn attrs(&self, py: Python<'_>) -> PyResult { + PyAttrs::read(py, Arc::clone(&self.handle), self.addr, &self.path) } /// Read with h5py indexing: integers, slices with positive steps, @@ -308,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 { @@ -317,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. @@ -330,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", @@ -361,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 5f7f51b..bfb6d57 100644 --- a/crates/clawhdf5-py/src/file.rs +++ b/crates/clawhdf5-py/src/file.rs @@ -1,13 +1,17 @@ //! PyFile — the main entry point for opening and creating HDF5 files. +use std::collections::HashMap; use std::path::PathBuf; use std::sync::{Arc, Mutex}; +use std::time::Duration; +use pyo3::exceptions::{PyNotImplementedError, PyValueError}; use pyo3::prelude::*; -use pyo3::types::PyList; +use pyo3::types::{PyDict, PyList}; use crate::attrs::PyAttrs; use crate::group::{PyGroup, ReadGroup, WriteGroupState, finalize_write_group}; +use crate::handle::Handle; use crate::{DatasetSpec, OwnedAttrValue, apply_dataset_spec, extract_numpy_data, to_py_err}; /// Internal state for write mode. @@ -23,8 +27,10 @@ struct WriteState { /// Mirrors the h5py.File interface: /// /// ```python -/// # Reading +/// # Reading, a local file or a URL (range requests, nothing downloaded +/// # up front) /// f = clawhdf5.File('data.h5', 'r') +/// f = clawhdf5.File('https://example.org/data.h5') /// ds = f['dataset'] /// f.close() /// @@ -44,49 +50,200 @@ enum FileInner { Write(WriteState), } +/// Whether `s` is a URL (`scheme://…`) rather than a path: the scheme is a +/// letter followed by letters, digits, `+`, `-` or `.` (RFC 3986). +fn is_url(s: &str) -> bool { + let Some((scheme, _)) = s.split_once("://") else { + return false; + }; + let mut chars = scheme.chars(); + chars.next().is_some_and(|c| c.is_ascii_alphabetic()) + && chars.all(|c| c.is_ascii_alphanumeric() || matches!(c, '+' | '-' | '.')) +} + +impl PyFile { + fn from_handle(handle: Arc, filename: String) -> Self { + let root = handle.root; + Self { + inner: Some(FileInner::Read(ReadGroup::new(handle, String::new(), root))), + filename, + } + } +} + #[pymethods] impl PyFile { /// Open or create an HDF5 file. /// /// Parameters: - /// path: file path + /// path: file path, or a URL (`http://`, `https://`, `s3://`, `gs://`, + /// `az://`; which schemes work depends on how the wheel was built) + /// to read the file remotely with default options (see `open_url`) /// mode: 'r' for read (default), 'w' for write #[new] #[pyo3(signature = (path, mode="r"))] fn new(py: Python<'_>, path: &str, mode: &str) -> PyResult { let filename = path.to_string(); - match mode { - "r" => { - let file = py.detach(|| { - crate::no_panic(|| clawhdf5_rs::File::open(path).map_err(to_py_err)) - })?; - Ok(Self { - inner: Some(FileInner::Read(root_group(Arc::new(file)))), - filename, - }) + if is_url(path) { + if mode != "r" { + return Err(PyValueError::new_err(format!( + "remote files are read-only: mode '{mode}' is not supported for a URL" + ))); } + let handle = Handle::open_url(py, path, &clawhdf5_remote::Options::default())?; + return Ok(Self::from_handle(handle, filename)); + } + 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 { - path: PathBuf::from(path), + // Absolute now: the file is written at close, possibly + // after the working directory changed. + path: std::path::absolute(path).unwrap_or_else(|_| PathBuf::from(path)), root_datasets: Vec::new(), root_attrs: Arc::new(Mutex::new(Vec::new())), groups: Vec::new(), })), }), - other => Err(PyErr::new::(format!( - "unsupported mode '{other}'; expected 'r' or 'w'" + other => Err(PyValueError::new_err(format!( + "unsupported mode '{other}'; expected 'r', 'r+', 'a' or 'w'" ))), } } + /// Open a remote file for reading, with options. + /// + /// The file is read through a block cache with range requests: opening + /// costs one request (it also fetches the first block), and a read + /// fetches only the blocks it needs. The GIL is released while waiting + /// on the network. + /// + /// Parameters (all optional): + /// block_size: bytes per cached block (default 1 MiB) + /// cache_size: byte budget of the block cache (default 64 MiB) + /// headers: dict of extra HTTP headers (e.g. Authorization), sent only + /// to the URL's own origin + /// retries: retries of a request that failed transiently (default 3) + /// timeout: seconds to connect and receive response headers (default 30) + /// allow_full_download: when the server ignores Range requests, + /// download the whole file once instead of failing (default False) + /// max_full_download: largest file such a download may fetch + /// (default 1 GiB) + /// require_validator: refuse a server that sends neither ETag nor + /// Last-Modified (default False) + /// max_redirects: redirects followed per request (default 5) + /// max_parallel: requests of one read in flight at once (default 8) + #[staticmethod] + #[allow(clippy::too_many_arguments)] + #[pyo3(signature = (url, *, block_size=None, cache_size=None, headers=None, retries=None, + timeout=None, allow_full_download=None, max_full_download=None, + require_validator=None, max_redirects=None, max_parallel=None))] + fn open_url( + py: Python<'_>, + url: &str, + block_size: Option, + cache_size: Option, + headers: Option>, + retries: Option, + timeout: Option, + allow_full_download: Option, + max_full_download: Option, + require_validator: Option, + max_redirects: Option, + max_parallel: Option, + ) -> PyResult { + let mut options = clawhdf5_remote::Options::default(); + if let Some(b) = block_size { + if b == 0 { + return Err(PyValueError::new_err("block_size must be positive")); + } + options.cache.block_size = b; + options.cache.coalesce_gap = b; + // The opening request fetches the first block, not 1 MiB. + options.http.first_request = b; + } + if let Some(c) = cache_size { + options.cache.capacity = c; + } + let http = &mut options.http; + if let Some(h) = headers { + http.headers = h.into_iter().collect(); + } + if let Some(r) = retries { + http.retries = r; + } + if let Some(t) = timeout { + if !(t.is_finite() && t > 0.0) { + return Err(PyValueError::new_err("timeout must be a positive number")); + } + http.timeout = Duration::from_secs_f64(t); + } + if let Some(a) = allow_full_download { + http.allow_full_download = a; + } + if let Some(m) = max_full_download { + http.max_full_download = m; + } + if let Some(v) = require_validator { + http.require_validator = v; + } + if let Some(r) = max_redirects { + http.max_redirects = r; + } + if let Some(p) = max_parallel { + if p == 0 { + return Err(PyValueError::new_err("max_parallel must be positive")); + } + http.max_parallel = p; + } + let handle = Handle::open_url(py, url, &options)?; + Ok(Self::from_handle(handle, url.to_string())) + } + + /// For a remote file, what its block cache has done so far (reads, + /// hits, misses, requests, bytes fetched, ...); `None` for a local file. + #[getter] + fn remote_stats<'py>(&self, py: Python<'py>) -> PyResult>> { + let Some(storage) = self.read_file()?.handle.remote_storage() else { + return Ok(None); + }; + let s = storage.stats(); + let d = PyDict::new(py); + d.set_item("reads", s.reads)?; + d.set_item("hits", s.hits)?; + d.set_item("misses", s.misses)?; + d.set_item("waits", s.waits)?; + d.set_item("requests", s.requests)?; + d.set_item("fetch_calls", s.fetch_calls)?; + d.set_item("bytes_fetched", s.bytes_fetched)?; + d.set_item("evictions", s.evictions)?; + d.set_item("cached_bytes", s.cached_bytes)?; + Ok(Some(d)) + } + /// Close the file. In write mode, this finalizes and writes the file. fn close(&mut self) -> PyResult<()> { let inner = self.inner.take().ok_or_else(|| { PyErr::new::("file is already closed") })?; match inner { - FileInner::Read(_) => Ok(()), + FileInner::Read(root) => { + root.handle.close(); + Ok(()) + } FileInner::Write(state) => finalize_write(state), } } @@ -121,7 +278,7 @@ impl PyFile { /// List the names of all children in the root group. fn keys(&self, py: Python<'_>) -> PyResult> { - let names = self.read_file()?.member_names()?; + let names = self.read_file()?.member_names(py)?; Ok(PyList::new(py, names)?.into_any().unbind()) } @@ -139,8 +296,8 @@ impl PyFile { self.keys(py)?.call_method0(py, "__iter__") } - fn __len__(&self) -> PyResult { - Ok(self.read_file()?.member_names()?.len()) + fn __len__(&self, py: Python<'_>) -> PyResult { + Ok(self.read_file()?.member_names(py)?.len()) } /// The root group's name, `/`. @@ -149,7 +306,31 @@ impl PyFile { "/" } - /// The path the file was opened with. + /// `'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 { &self.filename @@ -204,9 +385,9 @@ impl PyFile { /// Attribute access. In read mode, returns attributes of the root group. /// In write mode, returns a writable attrs handle. #[getter] - fn attrs(&self) -> PyResult { + fn attrs(&self, py: Python<'_>) -> PyResult { match self.inner.as_ref() { - Some(FileInner::Read(root)) => root.attrs(), + Some(FileInner::Read(root)) => root.attrs(py), Some(FileInner::Write(state)) => Ok(PyAttrs::from_write(Arc::clone(&state.root_attrs))), None => Err(PyErr::new::( "file is closed", @@ -216,9 +397,10 @@ impl PyFile { fn __repr__(&self) -> String { match &self.inner { - Some(FileInner::Read(root)) => { - format!("", root.file.as_bytes().len()) - } + Some(FileInner::Read(root)) => match root.handle.redacted_url() { + Some(url) => format!(""), + None => format!("", self.filename), + }, Some(FileInner::Write(s)) => { format!("", s.path.display()) } @@ -226,8 +408,8 @@ impl PyFile { } } - fn __contains__(&self, key: &str) -> PyResult { - Ok(self.read_file()?.contains(key)) + fn __contains__(&self, py: Python<'_>, key: &str) -> PyResult { + self.read_file()?.contains(py, key) } } @@ -248,6 +430,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", )), @@ -271,11 +460,6 @@ fn parse_compression( } } -fn root_group(file: Arc) -> ReadGroup { - let root = file.superblock().root_group_address; - ReadGroup::new(file, String::new(), root) -} - /// Build and write the HDF5 file from accumulated write state. fn finalize_write(state: WriteState) -> PyResult<()> { crate::no_panic(|| { @@ -309,6 +493,18 @@ fn finalize_write(state: WriteState) -> PyResult<()> { mod tests { use super::*; + #[test] + fn urls_and_paths() { + assert!(is_url("http://h/f.h5")); + assert!(is_url("s3://bucket/key.h5")); + assert!(is_url("git+https://x")); + assert!(!is_url("data.h5")); + assert!(!is_url("/tmp/a://b.h5")); + assert!(!is_url("dir/x://y")); + assert!(!is_url("1http://x")); + assert!(!is_url("://x")); + } + #[test] fn parse_gzip_compression() { assert_eq!(parse_compression(Some("gzip"), Some(6)).unwrap(), Some(6)); diff --git a/crates/clawhdf5-py/src/group.rs b/crates/clawhdf5-py/src/group.rs index 7585e57..4020f16 100644 --- a/crates/clawhdf5-py/src/group.rs +++ b/crates/clawhdf5-py/src/group.rs @@ -3,11 +3,12 @@ use std::collections::HashMap; use std::sync::{Arc, Mutex, OnceLock}; -use pyo3::exceptions::{PyIOError, PyKeyError, PyValueError}; +use pyo3::exceptions::{PyIOError, PyKeyError, PyNotImplementedError, PyOSError, PyValueError}; use pyo3::prelude::*; use pyo3::types::PyList; use crate::attrs::PyAttrs; +use crate::handle::Handle; use crate::{DatasetSpec, OwnedAttrValue, apply_dataset_spec, extract_numpy_data, node}; /// Shared state for a group being written. @@ -34,9 +35,9 @@ enum GroupInner { } impl PyGroup { - pub(crate) fn from_read(file: Arc, path: String, addr: u64) -> Self { + pub(crate) fn from_read(handle: Arc, path: String, addr: u64) -> Self { Self { - inner: GroupInner::Read(ReadGroup::new(file, path, addr)), + inner: GroupInner::Read(ReadGroup::new(handle, path, addr)), } } @@ -60,9 +61,10 @@ impl PyGroup { /// h5py). It keeps its own address and, once listed, its links, so looking /// up a child neither resolves the path from the root nor scans the group's /// links again: visiting every member of a large group is linear, not -/// quadratic. +/// quadratic. (Edits never add or remove links, so these stay valid in a +/// file open for editing.) pub(crate) struct ReadGroup { - pub file: Arc, + pub handle: Arc, pub path: String, pub addr: u64, /// Link name -> object address (soft links resolved), filled on first use. @@ -72,9 +74,9 @@ pub(crate) struct ReadGroup { } impl ReadGroup { - pub(crate) fn new(file: Arc, path: String, addr: u64) -> Self { + pub(crate) fn new(handle: Arc, path: String, addr: u64) -> Self { Self { - file, + handle, path, addr, links: OnceLock::new(), @@ -82,17 +84,14 @@ impl ReadGroup { } } - fn links(&self) -> PyResult<&HashMap> { + fn links(&self, py: Python<'_>) -> PyResult<&HashMap> { if let Some(links) = self.links.get() { return Ok(links); } - let entries = crate::no_panic(|| { - clawhdf5_format::group_v2::resolve_group_children( - self.file.as_bytes(), - self.file.superblock(), - self.addr, - ) - .map_err(|e| PyValueError::new_err(format!("{}: {e}", node::name(&self.path)))) + let (addr, path) = (self.addr, &self.path); + let entries = self.handle.with(py, |f| { + clawhdf5_format::group_v2::resolve_group_children_in(f.storage(), f.superblock(), addr) + .map_err(|e| node::format_err(path, e, PyValueError::new_err)) })?; let map = entries .into_iter() @@ -102,7 +101,7 @@ impl ReadGroup { } /// The path and address of `key` (a name, a relative or an absolute path). - fn locate(&self, key: &str) -> PyResult<(String, u64)> { + fn locate(&self, py: Python<'_>, key: &str) -> PyResult<(String, u64)> { let path = node::join(&self.path, key); let rel = if self.path.is_empty() { Some(path.as_str()) @@ -112,24 +111,24 @@ impl ReadGroup { path.strip_prefix(self.path.as_str()) .and_then(|r| r.strip_prefix('/')) }; - let addr = match rel { - // A direct child: the link table, when it has the name. - Some(name) if !name.is_empty() && !name.contains('/') => { - match self.links()?.get(name) { - Some(&a) => a, - None => node::resolve_from(&self.file, self.addr, name, &path)?, - } - } - Some(rel) => node::resolve_from(&self.file, self.addr, rel, &path)?, - None => node::address(&self.file, &path)?, - }; - Ok((path, addr)) + // A direct child: the link table, when it has the name. + if let Some(name) = rel.filter(|n| !n.is_empty() && !n.contains('/')) + && let Some(&a) = self.links(py)?.get(name) + { + return Ok((path, a)); + } + let addr = self.addr; + let found = self.handle.with(py, |f| match rel { + Some(rel) => node::resolve_from(f, addr, rel, &path), + None => node::address(f, &path), + })?; + Ok((path, found)) } /// `group[key]`. pub(crate) fn get_item(&self, py: Python<'_>, key: &str) -> PyResult> { - let (path, addr) = self.locate(key)?; - node::open(py, &self.file, path, addr) + let (path, addr) = self.locate(py, key)?; + node::open(py, &self.handle, path, addr) } /// `group.get(key, default)`. @@ -148,48 +147,57 @@ impl ReadGroup { } /// Names of the group's datasets and subgroups, sorted (h5py's order). - pub(crate) fn member_names(&self) -> PyResult<&[String]> { + pub(crate) fn member_names(&self, py: Python<'_>) -> PyResult<&[String]> { if let Some(m) = self.members.get() { return Ok(m); } - let mut names = Vec::new(); - for (name, &addr) in self.links()? { - let hdr = node::header_at(&self.file, addr, &node::join(&self.path, name))?; - if matches!( - node::kind(&hdr), - Some(node::Kind::Dataset | node::Kind::Group) - ) { - names.push(name.clone()); + let links = self.links(py)?; + let path = &self.path; + let mut names = self.handle.with(py, |f| { + let mut names = Vec::new(); + for (name, &addr) in links { + if matches!( + node::kind_at(f, addr, &node::join(path, name))?, + Some(node::Kind::Dataset | node::Kind::Group) + ) { + names.push(name.clone()); + } } - } + Ok(names) + })?; names.sort_by(|a, b| a.as_bytes().cmp(b.as_bytes())); Ok(self.members.get_or_init(|| names)) } - pub(crate) fn contains(&self, key: &str) -> bool { - self.locate(key) - .and_then(|(path, addr)| node::header_at(&self.file, addr, &path)) - .ok() - .and_then(|h| node::kind(&h)) - .is_some_and(|k| k != node::Kind::Datatype) + /// `key in group`: whether `key` names a dataset or group. A failed + /// read of the file (a network error) is raised, not `False`. + pub(crate) fn contains(&self, py: Python<'_>, key: &str) -> PyResult { + let found = self + .locate(py, key) + .and_then(|(path, addr)| self.handle.with(py, |f| node::kind_at(f, addr, &path))); + match found { + Ok(kind) => Ok(kind.is_some_and(|k| k != node::Kind::Datatype)), + Err(e) if e.is_instance_of::(py) => Err(e), + Err(_) => Ok(false), + } } pub(crate) fn values(&self, py: Python<'_>) -> PyResult>> { - self.member_names()? + self.member_names(py)? .iter() .map(|n| self.get_item(py, n)) .collect() } pub(crate) fn items(&self, py: Python<'_>) -> PyResult)>> { - self.member_names()? + self.member_names(py)? .iter() .map(|n| Ok((n.clone(), self.get_item(py, n)?))) .collect() } - pub(crate) fn attrs(&self) -> PyResult { - PyAttrs::read(Arc::clone(&self.file), self.addr, &self.path) + pub(crate) fn attrs(&self, py: Python<'_>) -> PyResult { + PyAttrs::read(py, Arc::clone(&self.handle), self.addr, &self.path) } } @@ -210,7 +218,7 @@ impl PyGroup { fn keys(&self, py: Python<'_>) -> PyResult> { match &self.inner { GroupInner::Read(g) => { - let list = PyList::new(py, g.member_names()?)?; + let list = PyList::new(py, g.member_names(py)?)?; Ok(list.into_any().unbind()) } GroupInner::Write(state) => { @@ -236,9 +244,9 @@ impl PyGroup { self.keys(py)?.call_method0(py, "__iter__") } - fn __len__(&self) -> PyResult { + fn __len__(&self, py: Python<'_>) -> PyResult { match &self.inner { - GroupInner::Read(g) => Ok(g.member_names()?.len()), + GroupInner::Read(g) => Ok(g.member_names(py)?.len()), GroupInner::Write(state) => Ok(state.lock().unwrap().datasets.len()), } } @@ -293,17 +301,28 @@ 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) -> PyResult { + fn attrs(&self, py: Python<'_>) -> PyResult { match &self.inner { - GroupInner::Read(g) => g.attrs(), + GroupInner::Read(g) => g.attrs(py), GroupInner::Write(state) => { let store = Arc::clone(&state.lock().unwrap().attrs); Ok(PyAttrs::from_write(store)) @@ -311,10 +330,10 @@ impl PyGroup { } } - fn __repr__(&self) -> String { + fn __repr__(&self, py: Python<'_>) -> String { match &self.inner { GroupInner::Read(g) => { - let n = g.member_names().map_or(0, |m| m.len()); + let n = g.member_names(py).map_or(0, |m| m.len()); format!("", node::name(&g.path)) } GroupInner::Write(state) => { @@ -324,9 +343,9 @@ impl PyGroup { } } - fn __contains__(&self, key: &str) -> PyResult { + fn __contains__(&self, py: Python<'_>, key: &str) -> PyResult { match &self.inner { - GroupInner::Read(g) => Ok(g.contains(key)), + GroupInner::Read(g) => g.contains(py, key), GroupInner::Write(state) => { let guard = state.lock().unwrap(); Ok(guard.datasets.iter().any(|d| d.name == key)) @@ -357,31 +376,6 @@ pub(crate) fn finalize_write_group( mod tests { use super::*; - #[test] - fn member_names_are_sorted() { - let mut b = clawhdf5_rs::FileBuilder::new(); - b.create_dataset("zeta").with_f64_data(&[1.0]); - b.create_dataset("alpha").with_f64_data(&[1.0]); - let mut g = b.create_group("mid"); - g.create_dataset("x").with_f64_data(&[1.0]); - let finished = g.finish(); - b.add_group(finished); - let bytes = b.finish().unwrap(); - let file = Arc::new(clawhdf5_rs::File::from_bytes(bytes).unwrap()); - let root = file.superblock().root_group_address; - let top = ReadGroup::new(Arc::clone(&file), String::new(), root); - assert_eq!(top.member_names().unwrap(), ["alpha", "mid", "zeta"]); - let (path, addr) = top.locate("mid").unwrap(); - assert_eq!(path, "mid"); - let mid = ReadGroup::new(Arc::clone(&file), path, addr); - assert_eq!(mid.member_names().unwrap(), ["x"]); - assert!(top.contains("mid/x")); - assert!(mid.contains("/alpha")); - assert!(mid.contains("x") && mid.contains("./x")); - assert!(!top.contains("nope")); - assert!(!mid.contains("alpha")); - } - #[test] fn finalize_group() { let state = WriteGroupState { diff --git a/crates/clawhdf5-py/src/handle.rs b/crates/clawhdf5-py/src/handle.rs new file mode 100644 index 0000000..4bb8364 --- /dev/null +++ b/crates/clawhdf5-py/src/handle.rs @@ -0,0 +1,230 @@ +//! The open file every object of a `File` shares. +//! +//! 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), 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, through the editor's own open file +//! rather than its path: 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::sync::atomic::{AtomicU64, Ordering}; +use std::sync::{Arc, Mutex, PoisonError, RwLock}; + +use clawhdf5_rs::{File, FileEditor}; +use pyo3::exceptions::PyOSError; +use pyo3::prelude::*; + +use crate::{panic_text, to_py_err}; + +/// Where the file's bytes come from. +pub(crate) enum Source { + /// A local file (memory-mapped). Its path is not kept: nothing reopens + /// it by path (see `open_editable`). + Local, + /// A URL, read through `clawhdf5-remote`'s block cache. + Remote { + url: String, + storage: Arc, + }, +} + +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, +} + +fn closed_after_failed_reopen() -> PyErr { + PyOSError::new_err("the file could not be reopened after an edit; open it again") +} + +impl Handle { + 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, + }) + } + + /// 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, 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 read through the editor's own file — never by path + /// again, so a later `os.chdir` or a rename or replacement of the path + /// cannot make reads (or the editor's plans) come from another file. + 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 = editor.reader().map_err(to_py_err)?; + Ok((file, editor)) + }) + })?; + Ok(Self::new(file, Source::Local, Some(editor))) + } + + /// A remote file (`http(s)://`, `s3://`, ...). + pub(crate) fn open_url( + py: Python<'_>, + url: &str, + options: &clawhdf5_remote::Options, + ) -> PyResult> { + let (file, storage) = py.detach(|| { + crate::no_panic(|| { + let storage = clawhdf5_remote::storage_for_url(url, options).map_err(remote_err)?; + let file = File::open_storage(storage.clone()).map_err(to_py_err)?; + Ok((file, storage)) + }) + })?; + Ok(Self::new( + file, + Source::Remote { + url: url.to_string(), + storage, + }, + None, + )) + } + + /// Run `f` on the file with the GIL released (a remote read may wait + /// on the network; other Python threads run meanwhile). `f` must not + /// touch Python. + pub(crate) fn with( + &self, + py: Python<'_>, + f: impl FnOnce(&File) -> PyResult + Send, + ) -> PyResult { + py.detach(|| self.with_detached(f)) + } + + /// [`with`](Self::with) for code that already runs without the GIL. + pub(crate) fn with_detached(&self, f: impl FnOnce(&File) -> PyResult) -> PyResult { + crate::no_panic(|| { + let guard = self.file.read().unwrap_or_else(PoisonError::into_inner); + let file = guard.as_ref().ok_or_else(closed_after_failed_reopen)?; + f(file) + }) + } + + /// 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 { + Source::Remote { storage, .. } => Some(storage), + Source::Local => None, + } + } + + /// The URL of a remote file, credentials and query values redacted. + pub(crate) fn redacted_url(&self) -> Option { + match &self.source { + Source::Remote { url, .. } => Some(clawhdf5_remote::redact_url(url)), + 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", + })); + }; + if matches!(self.source, Source::Remote { .. }) { + 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; + // Through the editor's file, not the path (see `open_editable`). + let reopened = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| ed.reader())); + 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 +/// (unreachable, a status, no range support, a changed file) is `OSError`, +/// a file that is not HDF5 is what `to_py_err` makes of it. +pub(crate) fn remote_err(e: clawhdf5_remote::Error) -> PyErr { + match e { + clawhdf5_remote::Error::Hdf5(e) => to_py_err(e), + other => PyOSError::new_err(other.to_string()), + } +} diff --git a/crates/clawhdf5-py/src/lib.rs b/crates/clawhdf5-py/src/lib.rs index 4055b82..4ae81f7 100644 --- a/crates/clawhdf5-py/src/lib.rs +++ b/crates/clawhdf5-py/src/lib.rs @@ -7,13 +7,22 @@ //! //! 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; mod node; mod select; @@ -63,7 +72,7 @@ fn _panic_for_test() -> PyResult<()> { /// Convert a `clawhdf5_rs::Error` into a `PyErr`. /// /// Maps different error variants to more specific Python exception types: -/// - I/O errors -> `PyIOError` +/// - I/O errors, and failed reads of a remote file -> `PyIOError`/`PyOSError` /// - Format/parsing errors -> `PyValueError` /// - Missing dataset/path errors -> `PyKeyError` /// - Invalid arguments -> `PyValueError` @@ -73,6 +82,10 @@ pub(crate) fn to_py_err(e: clawhdf5_rs::Error) -> PyErr { use clawhdf5_rs::Error; match &e { Error::Io(_) => PyErr::new::(e.to_string()), + // A failed read of the storage: a network error on a remote file. + Error::Format(clawhdf5_format::error::FormatError::Storage(_)) => { + PyErr::new::(e.to_string()) + } Error::Format(_) => PyErr::new::(e.to_string()), Error::NotADataset(_) | Error::MissingMessage(_) => { PyErr::new::(e.to_string()) diff --git a/crates/clawhdf5-py/src/node.rs b/crates/clawhdf5-py/src/node.rs index fa9ede7..35e5dec 100644 --- a/crates/clawhdf5-py/src/node.rs +++ b/crates/clawhdf5-py/src/node.rs @@ -1,16 +1,24 @@ //! Resolving paths to objects in a file opened for reading. +//! +//! Everything here parses through `File::storage()` (the `clawhdf5_format` +//! `*_in` functions), never `File::as_bytes()`, so it works the same on a +//! memory-mapped local file and on a remote one; and it runs inside +//! `Handle::with`, without the GIL. use std::sync::Arc; use clawhdf5_format::attribute::AttributeMessage; use clawhdf5_format::dataspace::{Dataspace, DataspaceType}; +use clawhdf5_format::error::FormatError; use clawhdf5_format::message_type::MessageType; use clawhdf5_format::object_header::ObjectHeader; -use pyo3::exceptions::{PyKeyError, PyTypeError, PyValueError}; +use clawhdf5_rs::File; +use pyo3::exceptions::{PyKeyError, PyOSError, PyTypeError, PyValueError}; use pyo3::prelude::*; -use crate::dataset::PyDataset; +use crate::dataset::{DatasetMeta, PyDataset}; use crate::group::PyGroup; +use crate::handle::Handle; /// Join `key` onto the group path `base` the way h5py does: an absolute key /// starts from the root, a relative one from `base`. Paths are kept without @@ -33,42 +41,46 @@ pub(crate) fn name(path: &str) -> String { format!("/{path}") } +/// A format error met at `path`: a failed read of the storage (a network +/// error on a remote file) is an `OSError`, anything else `other(message)`. +pub(crate) fn format_err(path: &str, e: FormatError, other: fn(String) -> PyErr) -> PyErr { + let msg = format!("{}: {e}", name(path)); + match e { + FormatError::Storage(_) => PyOSError::new_err(msg), + _ => other(msg), + } +} + +fn value_err(msg: String) -> PyErr { + PyValueError::new_err(msg) +} + /// The address of the object at `path`, resolved from the root group. -pub(crate) fn address(file: &clawhdf5_rs::File, path: &str) -> PyResult { +pub(crate) fn address(file: &File, path: &str) -> PyResult { resolve_from(file, file.superblock().root_group_address, path, path) } /// The address of `rel` resolved from the group at `group` (`full` is the /// resulting path, for the error message). -pub(crate) fn resolve_from( - file: &clawhdf5_rs::File, - group: u64, - rel: &str, - full: &str, -) -> PyResult { +pub(crate) fn resolve_from(file: &File, group: u64, rel: &str, full: &str) -> PyResult { if rel.is_empty() { return Ok(group); } - crate::no_panic(|| { - clawhdf5_format::group_v2::resolve_path_from(file.as_bytes(), file.superblock(), group, rel) - .map_err(|e| { - PyKeyError::new_err(format!( - "Unable to open object (object '{}' doesn't exist): {e}", - name(full) - )) - }) - }) + clawhdf5_format::group_v2::resolve_path_from_in(file.storage(), file.superblock(), group, rel) + .map_err(|e| match e { + FormatError::Storage(_) => format_err(full, e, value_err), + e => PyKeyError::new_err(format!( + "Unable to open object (object '{}' doesn't exist): {e}", + name(full) + )), + }) } /// The object header at `addr` (the object at `path`). -pub(crate) fn header_at(file: &clawhdf5_rs::File, addr: u64, path: &str) -> PyResult { - crate::no_panic(|| { - let sb = file.superblock(); - let at = usize::try_from(addr) - .map_err(|_| PyValueError::new_err(format!("{}: address out of range", name(path))))?; - ObjectHeader::parse(file.as_bytes(), at, sb.offset_size, sb.length_size) - .map_err(|e| PyValueError::new_err(format!("{}: {e}", name(path)))) - }) +pub(crate) fn header_at(file: &File, addr: u64, path: &str) -> PyResult { + let sb = file.superblock(); + ObjectHeader::parse_in(file.storage(), addr, sb.offset_size, sb.length_size) + .map_err(|e| format_err(path, e, value_err)) } /// What an object header describes. @@ -96,29 +108,50 @@ pub(crate) fn kind(hdr: &ObjectHeader) -> Option { } } +/// The kind of the object at `addr`, from its header. +pub(crate) fn kind_at(file: &File, addr: u64, path: &str) -> PyResult> { + Ok(kind(&header_at(file, addr, path)?)) +} + +/// What opening an object found, read without the GIL. +enum Found { + Dataset(DatasetMeta), + Group, + Datatype, + Other, +} + /// Open the object at `addr` (whose path is `path`) as a `Dataset` or /// `Group`. Both keep the address, so later reads resolve nothing. pub(crate) fn open( py: Python<'_>, - file: &Arc, + handle: &Arc, path: String, addr: u64, ) -> PyResult> { - let hdr = header_at(file, addr, &path)?; - match kind(&hdr) { - Some(Kind::Dataset) => Ok(PyDataset::open(py, Arc::clone(file), path, addr, &hdr)? + let found = handle.with(py, |f| { + let hdr = header_at(f, addr, &path)?; + Ok(match kind(&hdr) { + Some(Kind::Dataset) => Found::Dataset(DatasetMeta::load(f, addr, &hdr, &path)?), + Some(Kind::Group) => Found::Group, + Some(Kind::Datatype) => Found::Datatype, + None => Found::Other, + }) + })?; + match found { + Found::Dataset(meta) => Ok(PyDataset::new(py, Arc::clone(handle), path, addr, meta) .into_pyobject(py)? .into_any() .unbind()), - Some(Kind::Group) => Ok(PyGroup::from_read(Arc::clone(file), path, addr) + Found::Group => Ok(PyGroup::from_read(Arc::clone(handle), path, addr) .into_pyobject(py)? .into_any() .unbind()), - Some(Kind::Datatype) => Err(PyTypeError::new_err(format!( + Found::Datatype => Err(PyTypeError::new_err(format!( "{}: committed (named) datatypes are not supported by clawhdf5", name(&path) ))), - None => Err(PyValueError::new_err(format!( + Found::Other => Err(PyValueError::new_err(format!( "{}: not a dataset, group or datatype", name(&path) ))), @@ -126,32 +159,26 @@ pub(crate) fn open( } /// The dataspace message of an object header. -pub(crate) fn dataspace(file: &clawhdf5_rs::File, hdr: &ObjectHeader) -> PyResult { - crate::no_panic(|| { - let sb = file.superblock(); - let msg = hdr - .messages - .iter() - .find(|m| m.msg_type == MessageType::Dataspace) - .ok_or_else(|| PyValueError::new_err("object has no dataspace message"))?; - let data = clawhdf5_format::shared_message::message_data( - file.as_bytes(), - msg, - sb.offset_size, - sb.length_size, - ) - .map_err(|e| PyValueError::new_err(e.to_string()))?; - Dataspace::parse(&data, sb.length_size).map_err(|e| PyValueError::new_err(e.to_string())) - }) +pub(crate) fn dataspace(file: &File, hdr: &ObjectHeader, path: &str) -> PyResult { + let sb = file.superblock(); + let msg = hdr + .messages + .iter() + .find(|m| m.msg_type == MessageType::Dataspace) + .ok_or_else(|| PyValueError::new_err("object has no dataspace message"))?; + let data = clawhdf5_format::shared_message::message_data_in( + file.storage(), + msg, + sb.offset_size, + sb.length_size, + ) + .map_err(|e| format_err(path, e, value_err))?; + Dataspace::parse(&data, sb.length_size).map_err(|e| format_err(path, e, value_err)) } /// The chunk shape of a chunked dataset (one entry per dataset dimension), /// or `None` for other layouts or a layout message that does not parse. -pub(crate) fn chunk_shape( - file: &clawhdf5_rs::File, - hdr: &ObjectHeader, - rank: usize, -) -> Option> { +pub(crate) fn chunk_shape(file: &File, hdr: &ObjectHeader, rank: usize) -> Option> { let sb = file.superblock(); let msg = hdr .messages @@ -179,24 +206,18 @@ pub(crate) fn is_null(space: &Dataspace) -> bool { /// The attributes of the object at `addr` (whose path is `path`), sorted by /// name (h5py's order). Attributes whose messages cannot be parsed are left /// out, as the facade's `attrs()` does. -pub(crate) fn attributes( - file: &clawhdf5_rs::File, - addr: u64, - path: &str, -) -> PyResult> { +pub(crate) fn attributes(file: &File, addr: u64, path: &str) -> PyResult> { let hdr = header_at(file, addr, path)?; - crate::no_panic(|| { - let sb = file.superblock(); - let (mut attrs, _errors) = clawhdf5_format::attribute::extract_attributes_tolerant( - file.as_bytes(), - &hdr, - sb.offset_size, - sb.length_size, - ) - .map_err(|e| PyValueError::new_err(format!("{}: {e}", name(path))))?; - attrs.sort_by(|a, b| a.name.as_bytes().cmp(b.name.as_bytes())); - Ok(attrs) - }) + let sb = file.superblock(); + let (mut attrs, _errors) = clawhdf5_format::attribute::extract_attributes_tolerant_in( + file.storage(), + &hdr, + sb.offset_size, + sb.length_size, + ) + .map_err(|e| format_err(path, e, value_err))?; + attrs.sort_by(|a, b| a.name.as_bytes().cmp(b.name.as_bytes())); + Ok(attrs) } #[cfg(test)] diff --git a/crates/clawhdf5-py/src/select.rs b/crates/clawhdf5-py/src/select.rs index 7d11f3c..88ac358 100644 --- a/crates/clawhdf5-py/src/select.rs +++ b/crates/clawhdf5-py/src/select.rs @@ -7,11 +7,12 @@ //! (negative from the end) drop their axis, slices must have a positive //! step, one `Ellipsis` fills the unmentioned axes, a single increasing list //! of integers may index one axis, and strings name compound fields. -//! Everything else (`None`/`np.newaxis`, boolean masks, several index lists) -//! is refused with the error h5py gives. +//! Everything else (`None`/`np.newaxis`, several index lists) is refused +//! with the error h5py gives; boolean masks, which h5py supports, raise +//! `NotImplementedError`. use clawhdf5_format::selection::Selection; -use pyo3::exceptions::{PyIndexError, PyTypeError, PyValueError}; +use pyo3::exceptions::{PyIndexError, PyNotImplementedError, PyTypeError, PyValueError}; use pyo3::prelude::*; use pyo3::types::{PyEllipsis, PySlice, PyString, PyTuple}; @@ -254,6 +255,17 @@ pub(crate) fn parse(key: &Bound<'_, PyAny>, dims: &[u64]) -> PyResult { } } + // A mask of the dataset's whole shape (`ds[ds[()] > 0]`). + if let [a] = args.as_slice() { + let np = key.py().import("numpy")?; + if a.is_instance(&np.getattr("ndarray")?)? + && a.getattr("dtype")?.getattr("kind")?.extract::()? == "b" + && a.getattr("ndim")?.extract::()? > 1 + && a.getattr("shape")?.extract::>()? == dims + { + return Err(mask_unsupported()); + } + } if args.iter().any(|a| a.is_none()) { return Err(PyTypeError::new_err( "Indexing with None (or np.newaxis) is not supported", @@ -332,6 +344,10 @@ pub(crate) fn parse(key: &Bound<'_, PyAny>, dims: &[u64]) -> PyResult { }) } +fn mask_unsupported() -> PyErr { + PyNotImplementedError::new_err("boolean mask indexing is not supported by clawhdf5") +} + fn parse_axis(py: Python<'_>, a: &Bound<'_, PyAny>, n: u64) -> PyResult { if a.is_none() { return Err(PyTypeError::new_err( @@ -379,8 +395,15 @@ fn parse_axis(py: Python<'_>, a: &Bound<'_, PyAny>, n: u64) -> PyResult { let arr = np.call_method1("asarray", (a,))?; let kind: String = arr.getattr("dtype")?.getattr("kind")?.extract()?; if kind == "b" { + // A mask along this axis (h5py supports them; clawhdf5 does + // not, for reads or writes: an unsupported operation). A mask + // of any other shape is a wrong key, as in h5py. + let shape: Vec = arr.getattr("shape")?.extract()?; + if shape == [n] { + return Err(mask_unsupported()); + } return Err(PyTypeError::new_err( - "Boolean mask indexing is not supported by clawhdf5", + "Boolean indexing array has incompatible shape", )); } let ndim: usize = arr.getattr("ndim")?.extract()?; diff --git a/crates/clawhdf5-py/tests/conftest.py b/crates/clawhdf5-py/tests/conftest.py index 967f1e0..88113e8 100644 --- a/crates/clawhdf5-py/tests/conftest.py +++ b/crates/clawhdf5-py/tests/conftest.py @@ -1,6 +1,10 @@ """Shared fixtures for the clawhdf5 Python binding tests.""" import os +import re +import threading +import time +from http.server import BaseHTTPRequestHandler, ThreadingHTTPServer import pytest @@ -16,3 +20,134 @@ def h5py(): pytest.fail("h5py is required (CLAWHDF5_REQUIRE_INTEROP=1) but not importable") pytest.skip("h5py not installed") return mod + + +# --------------------------------------------------------------------------- +# An HTTP server for remote reads +# --------------------------------------------------------------------------- + +_RANGE = re.compile(r"^bytes=(\d*)-(\d*)$") + + +class RangeServer: + """A static file server on 127.0.0.1, in a thread of this process, that + answers `Range: bytes=a-b` with 206 and `Content-Range` (the way S3 and + common web servers do), sends an ETag and honours `If-Match`. + + - `ranges=False`: ignores `Range` and answers 200 with the whole file, + like a server without range support. + - `down` (set by `close()`): hang up on every request. + - `delay`: seconds to wait before answering each request after the + first `delay_after` ones (a slow network). + - `log`: every request as `(method, path, range header)`. + """ + + def __init__(self, root, ranges=True): + self.root = str(root) + self.ranges = ranges + self.delay = 0.0 + self.delay_after = 0 + self.down = False + self.log = [] + self._lock = threading.Lock() + server = self + + class Handler(BaseHTTPRequestHandler): + protocol_version = "HTTP/1.1" + + def log_message(self, *args): # quiet + pass + + def do_HEAD(self): + self._serve(body=False) + + def do_GET(self): + self._serve(body=True) + + def _serve(self, body): + with server._lock: + server.log.append((self.command, self.path, self.headers.get("Range"))) + n = len(server.log) + if server.delay and n > server.delay_after: + time.sleep(server.delay) + if server.down: + # Hang up without an answer (open keep-alive + # connections outlive shutdown(), so close() sets this). + self.close_connection = True + return + path = os.path.join(server.root, self.path.lstrip("/").split("?")[0]) + if not os.path.isfile(path): + self.send_response(404) + self.send_header("Content-Length", "0") + self.end_headers() + return + with open(path, "rb") as fh: + data = fh.read() + st = os.stat(path) + etag = f'"{st.st_mtime_ns:x}-{st.st_size:x}"' + want = self.headers.get("If-Match") + if want is not None and want != etag and want != "*": + self.send_response(412) + self.send_header("Content-Length", "0") + self.end_headers() + return + rng = self.headers.get("Range") if server.ranges else None + m = _RANGE.match(rng.strip()) if rng else None + if m and (m.group(1) or m.group(2)): + size = len(data) + if m.group(1): + start = int(m.group(1)) + end = int(m.group(2)) if m.group(2) else size - 1 + else: + start = max(0, size - int(m.group(2))) + end = size - 1 + if start >= size: + self.send_response(416) + self.send_header("Content-Range", f"bytes */{size}") + self.send_header("Content-Length", "0") + self.end_headers() + return + end = min(end, size - 1) + part = data[start : end + 1] + self.send_response(206) + self.send_header("Content-Range", f"bytes {start}-{end}/{size}") + else: + part = data + self.send_response(200) + if server.ranges: + self.send_header("Accept-Ranges", "bytes") + self.send_header("ETag", etag) + self.send_header("Content-Length", str(len(part))) + self.send_header("Content-Type", "application/x-hdf5") + self.end_headers() + if body: + try: + self.wfile.write(part) + except (BrokenPipeError, ConnectionResetError): + pass + + self.httpd = ThreadingHTTPServer(("127.0.0.1", 0), Handler) + self.httpd.daemon_threads = True + self.port = self.httpd.server_address[1] + self.thread = threading.Thread(target=self.httpd.serve_forever, daemon=True) + self.thread.start() + + def url(self, name): + return f"http://127.0.0.1:{self.port}/{name}" + + def requests(self): + with self._lock: + return len(self.log) + + def close(self): + self.down = True + self.httpd.shutdown() + self.httpd.server_close() + + +@pytest.fixture +def range_server(tmp_path): + """A range-capable server over `tmp_path`.""" + server = RangeServer(tmp_path) + yield server + server.close() diff --git a/crates/clawhdf5-py/tests/test_edit.py b/crates/clawhdf5-py/tests/test_edit.py new file mode 100644 index 0000000..a49df60 --- /dev/null +++ b/crates/clawhdf5-py/tests/test_edit.py @@ -0,0 +1,940 @@ +"""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=" 30] = 0 + with pytest.raises(NotImplementedError, match="mask"): + f["i4"][np.ones(6, dtype=bool), 2] = 0 + assert snapshot(h5py, path) == before + h5dump_reads(path) + + +def test_read_only_files_and_modes(h5py, tmp_path): + path = str(tmp_path / "m.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(" 0.1, "the edit is too quick to tell" + assert gaps.max() < 0.5 * (t1 - t0), ( + f"the other thread stalled for {gaps.max():.3f} s of a {t1 - t0:.3f} s edit") diff --git a/crates/clawhdf5-py/tests/test_read_vs_h5py.py b/crates/clawhdf5-py/tests/test_read_vs_h5py.py index 6a3b6cb..d45dd61 100644 --- a/crates/clawhdf5-py/tests/test_read_vs_h5py.py +++ b/crates/clawhdf5-py/tests/test_read_vs_h5py.py @@ -178,15 +178,39 @@ def _write_fixture(h5py, path): g.attrs["depth"] = np.int8(3) -@pytest.fixture(scope="module") -def pair(h5py, tmp_path_factory): - path = str(tmp_path_factory.mktemp("h5") / "fixture.h5") +@pytest.fixture(scope="module", params=["local", "r+", "http", "http-1k-blocks"]) +def pair(request, h5py, tmp_path_factory): + """The fixture file through h5py and through clawhdf5: opened locally + (read-only, and for editing: a copy, since editing locks the file), and + over HTTP range requests (a local server in this process) with the + default 1 MiB blocks and with 1 KiB blocks, so every structure is read + through many small ranges.""" + import shutil + + from conftest import RangeServer + + root = tmp_path_factory.mktemp("h5") + path = str(root / "fixture.h5") _write_fixture(h5py, path) theirs = h5py.File(path, "r") - ours = clawhdf5.File(path, "r") + server = None + if request.param == "local": + ours = clawhdf5.File(path, "r") + elif request.param == "r+": + copy = str(root / "editable.h5") + shutil.copy(path, copy) + ours = clawhdf5.File(copy, "r+") + else: + server = RangeServer(root) + if request.param == "http": + ours = clawhdf5.File(server.url("fixture.h5")) + else: + ours = clawhdf5.File.open_url(server.url("fixture.h5"), block_size=1024) yield ours, theirs, path theirs.close() ours.close() + if server is not None: + server.close() def _all_datasets(h5py, f): @@ -416,7 +440,7 @@ def test_unsupported_types_are_errors_not_data(pair): def test_boolean_masks_are_refused(pair): ours, _, _ = pair - with pytest.raises(TypeError): + with pytest.raises(NotImplementedError, match="mask"): ours["num/le_i4_1d"][np.ones(37, dtype=bool)] diff --git a/crates/clawhdf5-py/tests/test_remote.py b/crates/clawhdf5-py/tests/test_remote.py new file mode 100644 index 0000000..dbb24b8 --- /dev/null +++ b/crates/clawhdf5-py/tests/test_remote.py @@ -0,0 +1,244 @@ +"""Remote files: `clawhdf5.File(url)` / `File.open_url(url, ...)` read over +HTTP range requests (clawhdf5-remote's block cache), against a server in this +process (conftest.RangeServer). Values are compared with h5py reading the +same file locally; the rest checks what the server saw (only the blocks a +read needs are fetched), the failure modes (no range support, a missing +file, a file that changes, a server that goes away: errors, never wrong +data), and that the GIL is released while a read waits on the network.""" + +import os +import sys +import threading +import time + +import numpy as np +import pytest + +import clawhdf5 +from conftest import RangeServer + + +def _write(h5py, path): + rng = np.random.default_rng(7) + with h5py.File(path, "w") as f: + f.create_dataset("contig", data=rng.standard_normal((400, 300))) + f.create_dataset( + "chunked", + data=rng.integers(0, 1000, size=(512, 512), dtype="= 2 + + +def test_a_small_read_fetches_only_its_blocks(h5py, remote_file): + """With 4 KiB blocks, opening and reading one chunk of a 1 MB chunked + dataset costs a handful of requests and a few blocks, not the file.""" + path, url, server = remote_file + size = os.path.getsize(path) + f = clawhdf5.File.open_url(url, block_size=4096) + opened = server.requests() + assert opened == 1, server.log + ds = f["chunked"] + got = ds[0:10, 0:10] + with h5py.File(path, "r") as theirs: + np.testing.assert_array_equal(got, theirs["chunked"][0:10, 0:10]) + stats = f.remote_stats + assert stats["bytes_fetched"] < size / 4, (stats, size) + assert server.requests() - opened <= 12, server.log + # A second read of the same region is served by the cache. + before = server.requests() + ds[0:10, 0:10] + assert server.requests() == before + assert f.remote_stats["hits"] > stats["hits"] + assert clawhdf5.File(str(path), "r").remote_stats is None + + +def test_server_without_range_support(h5py, tmp_path): + """A server that ignores Range answers 200 with the whole file: that is + an OSError by default, and a whole download when allowed.""" + path = tmp_path / "remote.h5" + _write(h5py, str(path)) + server = RangeServer(tmp_path, ranges=False) + try: + url = server.url("remote.h5") + with pytest.raises(OSError, match="range"): + clawhdf5.File(url) + with clawhdf5.File.open_url(url, allow_full_download=True) as ours, h5py.File(path, "r") as theirs: + np.testing.assert_array_equal(ours["chunked"][...], theirs["chunked"][...]) + np.testing.assert_array_equal(ours["contig"][5], theirs["contig"][5]) + with pytest.raises(OSError): + clawhdf5.File.open_url(url, allow_full_download=True, max_full_download=1000) + finally: + server.close() + + +def test_errors_are_oserrors(remote_file): + _, url, server = remote_file + with pytest.raises(OSError, match="404"): + clawhdf5.File(server.url("missing.h5")) + with pytest.raises(ValueError, match="read-only"): + clawhdf5.File(url, "r+") + with pytest.raises(ValueError, match="read-only"): + clawhdf5.File(url, "w") + with pytest.raises(OSError, match="unsupported URL"): + clawhdf5.File("nosuchscheme://x/y.h5") + with pytest.raises(ValueError): + clawhdf5.File.open_url(url, block_size=0) + with pytest.raises(TypeError): + clawhdf5.File.open_url(url, no_such_option=1) + + +def test_object_store_urls_need_their_features(): + """The default wheel has no S3/GCS/Azure clients (aws-lc-rs builds C): + such a URL is an OSError naming the build feature.""" + for url, feature in [("s3://bucket/k.h5", "s3"), ("gs://b/k.h5", "gcs"), ("az://c/k.h5", "azure")]: + try: + clawhdf5.File(url) + except OSError as e: + if "feature" in str(e): + assert f"`{feature}`" in str(e), str(e) + else: + pytest.fail(f"{url} opened") + + +def test_https_needs_the_https_feature(): + """The default wheel has no TLS stack (rustls needs ring, which builds C): + an https URL is an OSError that names the build feature.""" + with pytest.raises(OSError) as e: + clawhdf5.File("https://127.0.0.1:1/x.h5") + msg = str(e.value) + # Built with `--features https` the error is the refused connection. + assert "https" in msg or "connect" in msg.lower() or "refused" in msg.lower(), msg + + +def test_a_changed_file_is_an_error_not_mixed_data(h5py, remote_file): + path, url, _ = remote_file + f = clawhdf5.File.open_url(url, block_size=1024) + first = f["grp/small"][...] + # Rewrite the file with other values: new ETag, same name. + time.sleep(0.01) + with h5py.File(path, "w") as g: + g.create_dataset("contig", data=np.zeros((400, 300))) + with pytest.raises(OSError, match="changed"): + f["contig"][...] + np.testing.assert_array_equal(first, np.arange(10, dtype=" before + assert took >= 0.2, took + assert progress["n"] > 1000, progress + # Held across a 0.2 s request, the spinner would stall that long. + assert progress["worst"] < 0.1, (progress, took) + + +def test_a_clawhdf5_written_file_reads_the_same_remotely(tmp_path, range_server): + path = tmp_path / "ours.h5" + data = np.arange(3000, dtype=" std::path::PathBuf { + let path = dir.join(name); + std::fs::copy( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("../clawhdf5/tests/fixtures") + .join(name), + &path, + ) + .unwrap(); + path +} + +/// Zero extents on chunked datasets with no recorded maximum. The unfixed +/// editor left `chunk_zero_extent_no_maxshape.h5` (a 2.7.0-written file +/// resized to 1x0): a Fixed Array whose maximum, taken from the current +/// dimensions, has no chunks along one dimension, so every stride before it +/// is 0 — `h5rs check` panicked dividing by it and the next resize failed +/// with an internal error. Such a file must check clean and resize on; a +/// 2.7.0-written file taken through zero extents by the fixed editor must +/// check clean at every step and read the fill value where it grew. +#[test] +fn zero_extent_resizes_without_a_recorded_maximum() { + if !tools_ok() { + return; + } + let dir = tmpdir(); + let path = fixture(dir.path(), "chunk_zero_extent_no_maxshape.h5"); + check_tools(&path, true); + let mut ed = FileEditor::open(&path).unwrap(); + ed.resize("d", &[0, 0]).unwrap(); + ed.resize("z", &[0, 0]).unwrap(); + // Their maximum is now what the index was laid out by (1 x 0). + ed.resize("d", &[1, 0]).unwrap(); + assert!(matches!( + ed.resize("d", &[1, 1]), + Err(Error::InvalidArgument(_)) + )); + drop(ed); + check_tools(&path, true); + + let path = fixture(dir.path(), "chunked_no_maxshape_v2_7_0.h5"); + for shape in [[15, 15], [3, 2], [1, 1], [1, 0], [0, 0], [0, 20], [20, 20]] { + let mut ed = FileEditor::open(&path).unwrap(); + ed.resize("d", &shape).unwrap(); + ed.resize("z", &shape).unwrap(); + drop(ed); + check_tools(&path, true); + } + let f = File::open(&path).unwrap(); + for name in ["d", "z"] { + let d = f.dataset(name).unwrap(); + assert_eq!(d.shape().unwrap(), [20, 20]); + assert!(d.read_f32().unwrap().iter().all(|&v| v == 0.0), "{name}"); + } + assert_eq!( + py(&format!( + "import h5py\n\ + with h5py.File({:?}) as f:\n\ + \x20 print(int(abs(f['d'][()]).sum() + abs(f['z'][()]).sum()), f['d'].maxshape)", + path.to_str().unwrap() + )), + "0 (20, 20)" + ); +} diff --git a/crates/clawhdf5/src/edit/mod.rs b/crates/clawhdf5/src/edit/mod.rs index a5a3c1b..455c77a 100644 --- a/crates/clawhdf5/src/edit/mod.rs +++ b/crates/clawhdf5/src/edit/mod.rs @@ -65,7 +65,8 @@ const MSG_FLAG_DONTSHARE: u8 = 0x04; /// that may be mid-update. /// /// Every method is one self-contained edit: it re-reads the file's -/// metadata, applies the change, and syncs the file before returning. +/// metadata (from the file it holds open, never by path), applies the +/// change, and syncs the file before returning. /// /// # What it can change /// @@ -750,9 +751,17 @@ impl FileEditor { /// consistent: a metadata cache image, paged or persistent free-space /// management, a multi-file driver, a file another writer has marked /// open (superblock version 3 consistency flags). + /// + /// The path is only used to open the file: every edit is planned from + /// and written to the file opened here, even if the path is renamed, + /// replaced or (relative) resolved from another working directory + /// later. [`path`](Self::path) is the absolute path it had at open. pub fn open>(path: P) -> Result { - let path = path.as_ref().to_path_buf(); - let file = OpenOptions::new().read(true).write(true).open(&path)?; + let file = OpenOptions::new() + .read(true) + .write(true) + .open(path.as_ref())?; + let path = std::fs::canonicalize(path.as_ref())?; match file.try_lock() { Ok(()) => {} Err(TryLockError::WouldBlock) => { @@ -768,16 +777,81 @@ impl FileEditor { file, free: FreeList::default(), }; - let f = File::open(&ed.path)?; + let f = ed.plan_reader()?; check_editable(&f)?; Ok(ed) } - /// The file's path. + /// The file's absolute path when it was opened (it may have been + /// renamed since; the editor keeps editing the file it opened). pub fn path(&self) -> &Path { &self.path } + /// A reader over the file this editor holds, as last written: the file + /// opened by [`open`](Self::open), not whatever its path names now. + /// + /// It opens the file anew (read-only), so it does not share the + /// editor's lock and stays usable after the editor is dropped: on Linux + /// through `/proc/self/fd`, which reaches the held file even after its + /// path was renamed or replaced; elsewhere by the path the file had at + /// open, refused with [`Error::Io`] when that path no longer names the + /// held file (on Unix, compared by device and inode; Windows cannot + /// check). With the `mmap` feature the reader maps the file: edits + /// through the editor change the bytes it sees, so take a new reader + /// after each edit rather than reading through an old one while an edit + /// runs. + pub fn reader(&self) -> Result { + let dir = self.path.parent().map(Path::to_path_buf); + File::from_std_file(self.reopen()?, dir) + } + + /// A new read-only open file description of the held file (see + /// [`reader`](Self::reader)). + fn reopen(&self) -> Result { + #[cfg(target_os = "linux")] + { + use std::os::fd::AsRawFd; + let proc = format!("/proc/self/fd/{}", self.file.as_raw_fd()); + if let Ok(f) = std::fs::File::open(proc) { + return Ok(f); + } + } + let f = std::fs::File::open(&self.path)?; + #[cfg(unix)] + { + use std::os::unix::fs::MetadataExt; + let (a, b) = (self.file.metadata()?, f.metadata()?); + if (a.dev(), a.ino()) != (b.dev(), b.ino()) { + return Err(Error::Io(std::io::Error::other(format!( + "{} no longer names the file being edited (renamed or replaced)", + self.path.display() + )))); + } + } + Ok(f) + } + + /// A reader over the held file for planning an edit (dropped before the + /// edit writes). On Linux a new open file description through + /// `/proc/self/fd`: a mapping of a clone of the held descriptor would + /// share its `flock`, and a process forked meanwhile (any + /// `std::process::Command` on another thread) would briefly keep the + /// lock alive after the editor is dropped. Elsewhere a clone of the + /// held descriptor, which follows the file wherever its path goes. + fn plan_reader(&self) -> Result { + let dir = self.path.parent().map(Path::to_path_buf); + #[cfg(target_os = "linux")] + { + use std::os::fd::AsRawFd; + let proc = format!("/proc/self/fd/{}", self.file.as_raw_fd()); + if let Ok(f) = std::fs::File::open(proc) { + return File::from_std_file(f, dir); + } + } + File::from_std_file(self.file.try_clone()?, dir) + } + /// Bytes earlier edits of this editor freed that later ones can still /// reuse. pub fn reusable_bytes(&self) -> u64 { @@ -794,7 +868,7 @@ impl FileEditor { &mut self, op: impl FnOnce(&File, &mut Image<'_>) -> Result, ) -> Result { - let f = File::open(&self.path)?; + let f = self.plan_reader()?; check_editable(&f)?; let sb = f.superblock().clone(); let user_block = f.user_block_size(); @@ -877,7 +951,7 @@ impl FileEditor { /// the fill value (`H5D__chunk_prune_by_extent`). pub fn resize(&mut self, path: &str, shape: &[u64]) -> Result<(), Error> { self.edit(|f, img| { - let t = Target::load(f, path)?; + let mut t = Target::load(f, path)?; let dims = t.dims().to_vec(); if shape.len() != dims.len() { return Err(Error::InvalidArgument(format!( @@ -889,7 +963,12 @@ impl FileEditor { if shape == dims.as_slice() { return Ok(()); } - let max = t.ds.max_dimensions.clone().unwrap_or_else(|| dims.clone()); + // No maximum recorded means the current dimensions (see below). + let record_max = t.ds.max_dimensions.is_none(); + let max = + t.ds.max_dimensions + .get_or_insert_with(|| dims.clone()) + .clone(); for d in 0..dims.len() { if shape[d] > max[d] { return Err(Error::InvalidArgument(format!( @@ -928,7 +1007,36 @@ impl FileEditor { } put_uint(&mut dims_bytes[d * ls..], n, img.ls); } - hdr.patch(img, i, first, &dims_bytes)?; + if !record_max { + hdr.patch(img, i, first, &dims_bytes)?; + } else { + // No maximum recorded (clawhdf5's writer, for a dataset + // created without a maxshape). libhdf5 never writes such a + // dataspace: `H5S_set_extent_simple` records the maximum, + // equal to the dimensions when none is given. Reading one, + // libhdf5 takes the maximum to be the *current* dimensions + // (`H5S_extent_get_dims`), so changing them would also + // change the maximum the chunk index was built with — the + // Fixed Array linearises chunks by it — and move every + // existing chunk. Record the maximum libhdf5 would have + // written, the dimensions before this resize, so the index + // keeps its layout and the dataset can grow back to them. + let body_len = first + dims.len() * ls; + if body.len() < body_len || body[2] & !0x01 != 0 { + return Err(Error::Unsupported("dataspace message layout".into())); + } + let mut new_body = body[..first].to_vec(); + new_body[2] |= 0x01; + new_body.extend_from_slice(&dims_bytes); + let at = new_body.len(); + new_body.resize(at + dims.len() * ls, 0); + for (d, &n) in dims.iter().enumerate() { + put_uint(&mut new_body[at + d * ls..], n, img.ls); + } + let (flags, corder) = (hdr.msgs[i].flags, hdr.msgs[i].corder); + hdr.delete(img, i)?; + hdr.insert(img, MSG_DATASPACE, flags, &new_body, corder)?; + } let fill = fill_info(img, &hdr)?; hdr.finish(img)?; let expand = shape.iter().zip(&dims).any(|(n, o)| n > o); diff --git a/crates/clawhdf5/src/reader.rs b/crates/clawhdf5/src/reader.rs index 57b81da..1ef5f04 100644 --- a/crates/clawhdf5/src/reader.rs +++ b/crates/clawhdf5/src/reader.rs @@ -446,6 +446,38 @@ impl File { } } + /// A reader over an already open file (the file itself, not whatever + /// its path names now): mapped with the `mmap` feature, else read into + /// memory. `base_dir` resolves external Virtual Dataset sources. + pub(crate) fn from_std_file( + file: std::fs::File, + base_dir: Option, + ) -> Result { + #[cfg(feature = "mmap")] + let mut f = { + let reader = clawhdf5_io::MmapReader::from_file(file).map_err(Error::Io)?; + let (data, superblock) = FileData::new(Backing::Mmap(reader))?; + Self { + data, + superblock, + chunk_cache: ChunkCache::new(), + base_dir: None, + vds_resolver: None, + } + }; + #[cfg(not(feature = "mmap"))] + let mut f = { + use std::io::{Read, Seek, SeekFrom}; + let mut file = file; + let mut bytes = Vec::new(); + file.seek(SeekFrom::Start(0)).map_err(Error::Io)?; + file.read_to_end(&mut bytes).map_err(Error::Io)?; + Self::from_bytes(bytes)? + }; + f.base_dir = base_dir; + Ok(f) + } + /// Open an HDF5 file by reading it entirely into memory. /// /// This is the pre-mmap behaviour and is useful when memory-mapping is diff --git a/crates/clawhdf5/tests/edit_resize_interop.rs b/crates/clawhdf5/tests/edit_resize_interop.rs new file mode 100644 index 0000000..83b4dae --- /dev/null +++ b/crates/clawhdf5/tests/edit_resize_interop.rs @@ -0,0 +1,287 @@ +//! `FileEditor::resize` on chunked datasets whose dataspace records no +//! maximum dimensions, as clawhdf5's writer stored a dataset created without +//! a `maxshape` up to 2.7.0 (`fixtures/chunked_no_maxshape_v2_7_0.h5`). libhdf5 never writes such a dataspace (`H5S_set_extent_simple` +//! always records the maximum, equal to the dimensions when none is given), +//! and its Fixed Array chunk index linearises chunks by the maximum +//! dimensions. The editor therefore records the maximum libhdf5 would have +//! written (the dimensions the index was built with) before it changes the +//! current ones, so existing chunks stay where the index put them and the +//! dataset can grow back to its original extent. +//! +//! The writer now records the maximum too, so h5py can resize what it writes. +//! +//! Checked against a model of the expected values, with our reader and with +//! h5py (`CLAWHDF5_PYTHON`; skipped without it unless +//! `CLAWHDF5_REQUIRE_INTEROP=1`), on files clawhdf5 (old and new) and h5py +//! wrote. + +use std::path::{Path, PathBuf}; +use std::process::Command; + +use clawhdf5::{Error, File, FileBuilder, FileEditor}; + +fn python() -> String { + std::env::var("CLAWHDF5_PYTHON").unwrap_or_else(|_| "python3".to_string()) +} + +fn h5py_ok() -> bool { + let ok = Command::new(python()) + .args(["-c", "import h5py, numpy"]) + .output() + .is_ok_and(|o| o.status.success()); + if !ok { + assert!( + !std::env::var("CLAWHDF5_REQUIRE_INTEROP").is_ok_and(|v| v == "1"), + "CLAWHDF5_REQUIRE_INTEROP=1 but h5py/numpy is not available" + ); + eprintln!("SKIP (h5py part): h5py/numpy not available"); + } + ok +} + +fn py(script: &str) -> String { + let o = Command::new(python()) + .args(["-c", script]) + .output() + .expect("run python"); + assert!( + o.status.success(), + "python failed:\n{script}\nSTDERR: {}", + String::from_utf8_lossy(&o.stderr) + ); + String::from_utf8_lossy(&o.stdout).trim().to_string() +} + +/// Row-major values of a 2-D model after resizing `data` (shape `old`) to +/// `new`: kept elements keep their values, new ones are 0 (the fill value). +fn resized(data: &[f32], old: [u64; 2], new: [u64; 2]) -> Vec { + let mut out = vec![0f32; (new[0] * new[1]) as usize]; + for r in 0..old[0].min(new[0]) { + for c in 0..old[1].min(new[1]) { + out[(r * new[1] + c) as usize] = data[(r * old[1] + c) as usize]; + } + } + out +} + +/// Our reader and (when available) h5py read `expect` at `shape`. +fn check(path: &Path, name: &str, shape: [u64; 2], expect: &[f32], with_h5py: bool) { + let f = File::open(path).unwrap(); + let d = f.dataset(name).unwrap(); + assert_eq!(d.shape().unwrap(), shape); + assert_eq!( + d.read_f32().unwrap(), + expect, + "{name}: our reader at {shape:?}" + ); + if with_h5py { + let got = py(&format!( + "import h5py, numpy as np\n\ + with h5py.File({p:?}, 'r') as f:\n\ + \x20 d = f[{name:?}][()]\n\ + print(d.shape, ','.join(repr(float(x)) for x in d.ravel()))", + p = path.to_str().unwrap() + )); + let want = format!( + "({}, {}) {}", + shape[0], + shape[1], + expect + .iter() + .map(|x| format!("{:?}", f64::from(*x))) + .collect::>() + .join(",") + ); + assert_eq!(got, want.trim(), "{name}: h5py at {shape:?}"); + } +} + +/// Resize `name` (20 x 20, values 0..400) through a sequence of shrinks, +/// zero extents and growth back, checking every step. +fn run(path: &Path, name: &str, with_h5py: bool) { + let orig: Vec = (0..400).map(|i| i as f32).collect(); + let mut shape = [20u64, 20]; + let mut data = orig.clone(); + check(path, name, shape, &data, with_h5py); + for next in [ + [15, 15], + [3, 2], + [20, 20], + [1, 1], + [1, 0], + [0, 0], + [7, 20], + [20, 13], + [20, 20], + ] { + let mut ed = FileEditor::open(path).unwrap(); + ed.resize(name, &next).unwrap(); + drop(ed); + data = resized(&data, shape, next); + shape = next; + check(path, name, shape, &data, with_h5py); + } + // The maximum is the extent the dataset was created with. + let mut ed = FileEditor::open(path).unwrap(); + assert!(matches!( + ed.resize(name, &[21, 20]), + Err(Error::InvalidArgument(_)) + )); + drop(ed); + let f = File::open(path).unwrap(); + assert_eq!( + f.dataset(name).unwrap().max_dimensions().unwrap(), + Some(vec![20, 20]) + ); + drop(f); + // A shrink keeps the values it keeps. + let mut ed = FileEditor::open(path).unwrap(); + let vals: Vec = orig.iter().map(|v| v + 0.5).collect(); + ed.write_values(name, &clawhdf5::Selection::All, &vals) + .unwrap(); + ed.resize(name, &[15, 15]).unwrap(); + drop(ed); + check( + path, + name, + [15, 15], + &resized(&vals, [20, 20], [15, 15]), + with_h5py, + ); +} + +fn fixture(dir: &Path, name: &str) -> PathBuf { + let path = dir.join(name); + std::fs::copy( + Path::new(env!("CARGO_MANIFEST_DIR")) + .join("tests/fixtures") + .join(name), + &path, + ) + .unwrap(); + path +} + +/// Files clawhdf5 2.7.0 wrote: a Fixed Array index (Single Chunk for `s`) +/// and a dataspace with no maximum. Shrinking scrambled the values +/// (released in 2.7.0's `FileEditor`, PR #18). +#[test] +fn resize_without_stored_maxshape_keeps_values() { + let with_h5py = h5py_ok(); + for name in ["d", "z"] { + let dir = tempfile::tempdir().unwrap(); + let path = fixture(dir.path(), "chunked_no_maxshape_v2_7_0.h5"); + run(&path, name, with_h5py); + } + // A single-chunk dataset and an empty one keep their extents as maxima. + let dir = tempfile::tempdir().unwrap(); + let path = fixture(dir.path(), "chunked_no_maxshape_v2_7_0.h5"); + let mut ed = FileEditor::open(&path).unwrap(); + ed.resize("s", &[2, 4]).unwrap(); + ed.resize("s", &[3, 4]).unwrap(); + assert!(matches!( + ed.resize("s", &[4, 4]), + Err(Error::InvalidArgument(_)) + )); + assert!(matches!( + ed.resize("e", &[1, 5]), + Err(Error::InvalidArgument(_)) + )); + ed.resize("e", &[0, 3]).unwrap(); + drop(ed); + let f = File::open(&path).unwrap(); + let s = f.dataset("s").unwrap(); + assert_eq!(s.max_dimensions().unwrap(), Some(vec![3, 4])); + let mut want: Vec = (0..12).collect(); + want[8..].fill(0); + assert_eq!(s.read_i32().unwrap(), want); + assert_eq!( + f.dataset("e").unwrap().max_dimensions().unwrap(), + Some(vec![0, 5]) + ); +} + +fn written(path: &Path, deflate: bool) { + let data: Vec = (0..400).map(|i| i as f32).collect(); + let mut b = FileBuilder::new(); + let d = b + .create_dataset("d") + .with_f32_data(&data) + .with_shape(&[20, 20]) + .with_chunks(&[6, 6]); + if deflate { + d.with_deflate(4); + } + b.write(path).unwrap(); +} + +/// clawhdf5's writer now records the maximum, as libhdf5 does. +#[test] +fn resize_file_written_without_maxshape_keeps_values() { + let with_h5py = h5py_ok(); + for deflate in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cw.h5"); + written(&path, deflate); + let f = File::open(&path).unwrap(); + assert_eq!( + f.dataset("d").unwrap().max_dimensions().unwrap(), + Some(vec![20, 20]) + ); + drop(f); + run(&path, "d", with_h5py); + } +} + +/// h5py resizing a file clawhdf5 wrote without a maxshape keeps its values +/// (it scrambled them while the writer recorded no maximum). +#[test] +fn h5py_resizes_what_clawhdf5_writes() { + if !h5py_ok() { + return; + } + for deflate in [false, true] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("cw.h5"); + written(&path, deflate); + let out = py(&format!( + "import h5py, numpy as np\n\ + exp = np.arange(400, dtype='f4').reshape(20, 20)\n\ + with h5py.File({p:?}, 'r+') as f:\n\ + \x20 f['d'].resize((15, 15))\n\ + \x20 ok = np.array_equal(f['d'][()], exp[:15, :15])\n\ + \x20 f['d'].resize((20, 20))\n\ + \x20 back = f['d'][()]\n\ + want = np.zeros((20, 20), 'f4'); want[:15, :15] = exp[:15, :15]\n\ + print(ok and np.array_equal(back, want))", + p = path.to_str().unwrap() + )); + assert_eq!(out, "True"); + let mut want = vec![0f32; 400]; + for r in 0..15 { + for c in 0..15 { + want[r * 20 + c] = (r * 20 + c) as f32; + } + } + check(&path, "d", [20, 20], &want, false); + } +} + +/// h5py's files record the maximum; the same sequence must hold. +#[test] +fn resize_h5py_file_without_maxshape_keeps_values() { + if !h5py_ok() { + return; + } + for libver in ["earliest", "v110", "latest"] { + let dir = tempfile::tempdir().unwrap(); + let path = dir.path().join("hp.h5"); + py(&format!( + "import h5py, numpy as np\n\ + with h5py.File({p:?}, 'w', libver=({libver:?}, 'latest')) as f:\n\ + \x20 f.create_dataset('d', data=np.arange(400, dtype='f4').reshape(20, 20), chunks=(6, 6))", + p = path.to_str().unwrap() + )); + run(&path, "d", true); + } +} diff --git a/crates/clawhdf5/tests/edit_tests.rs b/crates/clawhdf5/tests/edit_tests.rs index 9190a2c..1f83d74 100644 --- a/crates/clawhdf5/tests/edit_tests.rs +++ b/crates/clawhdf5/tests/edit_tests.rs @@ -162,3 +162,59 @@ fn shrink_then_grow_reads_fill() { raw[..4].fill(0.5); assert_eq!(f.dataset("raw").unwrap().read_f64().unwrap(), raw); } + +/// The editor plans every edit from the file it holds open, never by +/// re-opening its path: with the path renamed away and another file put in +/// its place, edits go to the held file, planned from its own metadata, +/// and the file now at the path is untouched (planning from it and writing +/// into the held file corrupted the held one). +#[test] +fn edits_go_to_the_file_held_not_the_path() { + let dir = tempfile::tempdir().unwrap(); + let a = dir.path().join("a.h5"); + let b = dir.path().join("b.h5"); + let mut fb = FileBuilder::new(); + fb.create_dataset("x") + .with_i32_data(&[0; 10]) + .with_shape(&[10]); + fb.create_dataset("big") + .with_f64_data(&[1.5; 5000]) + .with_shape(&[5000]); + fb.set_attr("title", AttrValue::String("a".into())); + fb.write(&a).unwrap(); + let mut fb = FileBuilder::new(); + fb.create_dataset("pad") + .with_f64_data(&[2.5; 3000]) + .with_shape(&[3000]); + fb.create_dataset("x") + .with_i32_data(&[500; 10]) + .with_shape(&[10]); + fb.write(&b).unwrap(); + + let mut ed = FileEditor::open(&a).unwrap(); + assert!(ed.path().is_absolute()); + let moved = dir.path().join("moved.h5"); + std::fs::rename(&a, &moved).unwrap(); + std::fs::rename(&b, &a).unwrap(); + let other = std::fs::read(&a).unwrap(); + + ed.write_values("x", &Selection::All, &[7i32; 10]).unwrap(); + let vals: Vec = (0..50).map(f64::from).collect(); + ed.set_attr("/", "note", &AttrValue::F64Array(vals.clone())) + .unwrap(); + // The editor's own reader sees the held file. + let r = ed.reader().unwrap(); + assert_eq!(r.dataset("x").unwrap().read_i32().unwrap(), [7; 10]); + assert!(r.dataset("pad").is_err()); + drop(r); + drop(ed); + + assert!( + std::fs::read(&a).unwrap() == other, + "the file at the path changed" + ); + let f = File::open(&moved).unwrap(); + assert_eq!(f.dataset("x").unwrap().read_i32().unwrap(), [7; 10]); + assert_eq!(f.dataset("big").unwrap().read_f64().unwrap(), [1.5; 5000]); + assert!(matches!(f.root().attr("note").unwrap(), Some(AttrValue::F64Array(v)) if v == vals)); +} diff --git a/crates/clawhdf5/tests/fixtures/chunk_zero_extent_no_maxshape.h5 b/crates/clawhdf5/tests/fixtures/chunk_zero_extent_no_maxshape.h5 new file mode 100644 index 0000000..fa1e0ef Binary files /dev/null and b/crates/clawhdf5/tests/fixtures/chunk_zero_extent_no_maxshape.h5 differ diff --git a/crates/clawhdf5/tests/fixtures/chunked_no_maxshape_v2_7_0.h5 b/crates/clawhdf5/tests/fixtures/chunked_no_maxshape_v2_7_0.h5 new file mode 100644 index 0000000..7ecdfe7 Binary files /dev/null and b/crates/clawhdf5/tests/fixtures/chunked_no_maxshape_v2_7_0.h5 differ diff --git a/crates/clawhdf5/tests/plugin_filters_interop.rs b/crates/clawhdf5/tests/plugin_filters_interop.rs index f157ff5..9b41b2a 100644 --- a/crates/clawhdf5/tests/plugin_filters_interop.rs +++ b/crates/clawhdf5/tests/plugin_filters_interop.rs @@ -966,7 +966,10 @@ fn skipped_optional_filters_are_masked_as_libhdf5_masks_them() { /// Files whose chunks all compress are written exactly as before optional /// filters could be skipped: every mask is 0 and nothing else changed. The -/// hashes are of the files the writer produced before that change. +/// hashes are of the files the writer produced before that change, except +/// that a chunked dataset without a maxshape now records its maximum +/// dimensions (8 bytes per dimension; `lzf_fixed`, `lzf_single`, and +/// `blosc_fixed`). #[cfg(feature = "lzf")] #[test] fn files_whose_chunks_all_compress_are_unchanged() { @@ -985,7 +988,7 @@ fn files_whose_chunks_all_compress_are_unchanged() { .with_chunks(&[500]) .with_lzf(); }, - (3965, 449169442), + (3973, 452644487), ), ( "lzf_ea_noshuffle", @@ -1017,7 +1020,7 @@ fn files_whose_chunks_all_compress_are_unchanged() { .with_chunks(&[3000]) .with_lzf(); }, - (546, 690805477), + (554, 1394027497), ), ]; #[cfg(feature = "blosc")] @@ -1029,7 +1032,7 @@ fn files_whose_chunks_all_compress_are_unchanged() { .with_chunks(&[1024]) .with_blosc(BloscCodec::Lz4, 5, BloscShuffle::Byte); }, - (2776, 4278611376), + (2784, 1180133244), )); for (name, build, want) in &cases { let mut fb = clawhdf5::FileBuilder::new(); diff --git a/docs/design/range-reads.md b/docs/design/range-reads.md index e0d10ef..fbb363e 100644 --- a/docs/design/range-reads.md +++ b/docs/design/range-reads.md @@ -12,6 +12,10 @@ done on branch `feat/p3-m5-swmr-reader`, with its own design in [`swmr.md`](swmr.md) (see the M5 status below). M4 (wasm) is next. Every count in §1–§2 was +object stores) and URLs in `h5rs` (see the M3 status below); the Python +bindings followed on branch `feat/p3-python-remote-edit` (2026-09-27), +which completes M3. M4 (wasm) is next. Every count in §1–§2 was + change. Progress: M1, first part (the `Storage` trait and the metadata parsers listed in `CHANGELOG.md` under "Range reads, milestone M1") is done; group B-tree v2 lookups, dense groups and the facade are not converted yet. @@ -477,7 +481,7 @@ fast path within benchmark noise. a request counter exposed for tests and users. - Python bindings: `clawhdf5.File("s3://…")` / `https://` through it. - *Status 2026-09-26:* done on branch `feat/p3-m3-remote`, except the - Python bindings, with these choices: + Python bindings (done 2026-09-27, below), with these choices: - A new crate, `clawhdf5-remote`, instead of a `remote` feature of `clawhdf5-io`: `open_url` returns a `clawhdf5::File`, and `clawhdf5-io` sits below the facade. @@ -514,6 +518,17 @@ fast path within benchmark noise. same work without a cache: 141 936 requests. Per file: A lists in 2 requests (§2 predicted 2 blocks of 1 MiB), B in 1, C in 7 (its whole 6.4 MB: 35 001 object headers spread over the file). +- *Status 2026-09-27, Python bindings:* done on branch + `feat/p3-python-remote-edit`. `clawhdf5.File(url)` and + `File.open_url(url, **options)` (cache and HTTP options) go through + `clawhdf5_remote::storage_for_url`; the default wheel is plain HTTP (no + C), `https`/`s3`/`gcs`/`azure` are build features. The bindings' own + parsing (path lookups, headers, attributes, listings, the global heap) + moved from `File::as_bytes` to `File::storage()` and the `*_in` + functions, and every file access, metadata included, runs with the GIL + released. Checked by running the whole read-vs-h5py suite over an + in-process range server (1 MiB and 1 KiB blocks) and by request counts + in `crates/clawhdf5-py/tests/test_remote.py`. **M4 — wasm lazy loading (1–2 weeks).** - `clawhdf5-wasm`: `openUrl(url) -> Promise` backed by `fetch` with a diff --git a/docs/known-issues.md b/docs/known-issues.md index e24e076..d4136d7 100644 --- a/docs/known-issues.md +++ b/docs/known-issues.md @@ -30,6 +30,33 @@ h5py's SWMR reader). A file still being written is read with `File::open_swmr` (see `docs/design/swmr.md`); `File::open` maps the file at its length at open and is not meant for files that change while open. +## Shrinking a chunked dataset with no recorded maximum scrambled it + +**Status:** fixed 2026-09-27, before any release (`FileEditor::resize` +shipped on main in PR #18, a4c2ace). Files the writer produced before the +fix still lack the maximum; see *Existing files*. + +clawhdf5's writer stored no maximum dimensions for a chunked dataset +created without a `maxshape` (libhdf5 always stores one, equal to the +dimensions when none is given). With none recorded, the maximum is the +current dimensions (`H5S_extent_get_dims`), and a Fixed Array chunk index +places chunks by the maximum. `FileEditor::resize` changed only the +current dimensions, so a shrink moved every existing chunk and every +reader returned wrong values; after a shrink the dataset could not grow +back. Found by the review of the Python editing work. + +**Fix:** before the first resize of such a dataset the editor records the +maximum libhdf5 would have written (the dimensions the index was built +with); the writer now records it for every chunked dataset. **Test:** +`crates/clawhdf5/tests/edit_resize_interop.rs`. **Existing files:** +datasets written before the fix have no recorded maximum. The fixed +editor handles them; **libhdf5 (h5py `Dataset.resize`, `H5Dset_extent`) +does not** — it scrambles them the same way, and lets them grow past +their Fixed Array. Resize them once with a fixed `FileEditor` (a resize +to the same shape changes nothing; shrink and grow back) before letting +libhdf5 resize them. A dataset already shrunk by the unfixed +editor holds misplaced chunks; rewrite it from a good copy. + ## Fletcher-32 checksums disagreed with libhdf5 on about 1 chunk in 32768 **Status:** fixed 2026-09-26, after v2.7.0. **Every release (v2.1.0 to @@ -139,6 +166,57 @@ space it leaves is too small for its next, larger version. **No journal.** A crash while an edit patches existing structures can leave the file inconsistent; see the `FileEditor` documentation. +**Renamed files outside Linux.** Edits always go to the file the editor +opened. `FileEditor::reader()` (which the Python `'r+'` handle reads +through) reopens it through `/proc/self/fd` on Linux; elsewhere it reopens +the path, and after the path was renamed or replaced it fails (on Unix; +Windows cannot tell and would read whatever the path names). + +## Python in-place editing (`clawhdf5.File(path, 'r+')`) limits + +**Status:** open (added 2026-09-27). The Python bindings edit through +`FileEditor`, so its limits above apply, raised as `NotImplementedError` +before anything is written. On top of them: + +- **No new or deleted objects:** `create_dataset`/`create_group` in an + `'r+'` file, `del f[name]` and `del obj.attrs[name]` raise + `NotImplementedError` (the editor changes values, shapes and + attributes only). Mode `'a'` works on an existing file only. +- **Not writable from Python:** compound fields by name (`ds['x'] = …`; + whole elements of the same structured dtype are), HDF5 array-type + elements, variable-length data, strings padded with spaces or + NUL-terminated (libhdf5 converts those differently from numpy; NUL-padded + ones, h5py's, are writable), compounds containing such strings, null + dataspaces, index-list writes of more than 2²² elements (write them + in slices), and boolean-mask keys (`ds[mask] = v`, and mask reads; + h5py supports both). +- **`str` attributes are fixed-length UTF-8**, where h5py writes + variable-length strings: h5py reads them back as `bytes` + (`numpy.bytes_`), not `str`. +- **Numeric conversion follows libhdf5's native-order results, not its + bugs.** Arrays are converted as libhdf5 converts them (integers + saturate, floats are truncated toward zero and clipped), checked value by + value against h5py 3.16 / HDF5 2.0 in + `crates/clawhdf5-py/tests/test_edit.py::test_numeric_conversions_match_h5py` + (2026-09-27, tank). Where libhdf5 itself is inconsistent, clawhdf5 + differs from h5py on purpose: + - NaN into an integer dataset is a `ValueError` (libhdf5 stores 0, the + minimum or 2⁶³ depending on the type); + - when the dataset or the array is not in native byte order, libhdf5's + "soft" conversions store a float in (-1, 0) as the integer minimum and + wrap an unsigned value too large for the signed type of the same size + (65535 → -1); clawhdf5 gives 0 and the maximum, as libhdf5 does in + native order; + - libhdf5's native casts that are undefined in C: half floats into + unsigned integers (-1 → 65535, +inf → 0), half-float ±inf into signed + integers (→ minimum), a float equal to the integer maximum rounded up + in its precision (`float32(2**31 - 1)` into `int32`, `float64(2**64 - + 1)` into `uint64`: → minimum or 0); clawhdf5 saturates; + - a double in (65504, 65520) into a half float: libhdf5 stores infinity, + clawhdf5 (numpy) rounds to 65504 as IEEE 754 does. +- **Each edit reopens the file** (a new memory map) so that reads see it; + reads from other threads wait while an edit is written. + ## Selection reads that decode more than the selection **Status:** open (documented 2026-09-26). `Dataset::read_selection` (and so @@ -867,9 +945,9 @@ cache, but: `read_*_zerocopy`) need the file in memory and answer `FormatError::ContiguousStorageRequired` otherwise; `File::as_bytes()` panics for such a file (`File::contiguous_bytes()` is the fallible form). - `LazyFile`, `MmapFile` and the Python and wasm bindings still read a - whole file (`h5rs` reads through `File::storage`, and takes URLs with its - `remote` feature). + `LazyFile`, `MmapFile` and the wasm bindings still read a whole file + (`h5rs` and the Python bindings read through `File::storage`, and take + URLs: `h5rs` with its `remote` feature, Python with `clawhdf5.File(url)`). - The file's length is read once, at open: a growing file (SWMR) is not followed (milestone M5). A remote file is pinned at open, so one that grows is `RemoteError::FileChanged`. @@ -885,9 +963,12 @@ cache, but: **Status:** open (added 2026-09-26, milestone M3 of `docs/design/range-reads.md`). -- **Python and the browser cannot open URLs yet.** `clawhdf5.File` (PyO3) - parses through `File::as_bytes`, which a remote file does not have; the - wasm reader's `openUrl` is milestone M4. +- **The browser cannot open URLs yet**: the wasm reader's `openUrl` is + milestone M4. Python can (`clawhdf5.File(url)`, since 2026-09-27), but + the default wheel reads plain `http://` only: `https://` needs a wheel + built with `--features https` (rustls with ring, which compiles C), and + `s3://`, `gs://`, `az://` the `s3`, `gcs`, `azure` features (aws-lc-rs). + The Python tests run against an in-process `http.server` only. - **The block size is fixed** (1 MiB unless `CacheConfig` says otherwise). The design's policy of using a paged file's page size as the block size is not implemented, and only the first block is read ahead. diff --git a/scripts/ci-test.sh b/scripts/ci-test.sh index 4f678ad..7ba09a2 100755 --- a/scripts/ci-test.sh +++ b/scripts/ci-test.sh @@ -131,14 +131,17 @@ run_step "cargo clippy (h5rs remote)" cargo clippy \ # js-sys (clawhdf5-wasm's bindings to JavaScript) builds no C. # clawhdf5-remote is checked by default (plain HTTP) and with its # object-store feature, and h5rs with URL support (remote); the https -# (ring) and s3/gcs/azure (aws-lc-rs) features build C and are opt-in. +# (ring) and s3/gcs/azure (aws-lc-rs) features build C and are opt-in. The +# Python bindings (clawhdf5-py, remote reads over plain HTTP) are checked too: +# their https/s3/gcs/azure features are opt-in for the same reason. no_c_in_default_build() { local entry crate features found=0 for entry in clawhdf5-format clawhdf5-io clawhdf5-filters clawhdf5 \ clawhdf5-agent clawhdf5-ann clawhdf5-accel clawhdf5-netcdf4 clawhdf5-cli \ clawhdf5-tools \ clawhdf5-wasm \ - clawhdf5-remote clawhdf5-remote:object-store clawhdf5-tools:remote; do + clawhdf5-remote clawhdf5-remote:object-store clawhdf5-tools:remote \ + clawhdf5-py; do crate=${entry%%:*} features=() [ "$entry" != "$crate" ] && features=(--features "${entry#*:}") @@ -269,7 +272,10 @@ python_package() { -i "$PYTHON" \ --out "$out/wheel" || return 1 "$PYTHON" -m pip install --quiet --no-deps --target "$out/site" "$out"/wheel/*.whl || return 1 - PYTHONPATH="$out/site" "$PYTHON" -m pytest -q -p no:cacheprovider \ + # The editing tests run `h5rs check` on every file they edit. + cargo build -q -p clawhdf5-tools || return 1 + CLAWHDF5_H5RS="${CARGO_TARGET_DIR:-$root/target}/debug/h5rs" \ + PYTHONPATH="$out/site" "$PYTHON" -m pytest -q -p no:cacheprovider \ "$root/crates/clawhdf5-py/tests" } if "$PYTHON" -m maturin --version >/dev/null 2>&1 && "$PYTHON" -c "import pytest" >/dev/null 2>&1; then