Skip to content

Commit dc44cb2

Browse files
authored
Merge pull request #32 from cellgeni/fix/copy-null-and-empty-chunks
Copy nulls, compression and empty arrays across backends
2 parents cad694c + 59c6b03 commit dc44cb2

8 files changed

Lines changed: 279 additions & 48 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,32 @@
33
Notable changes to `adata-cli`. Versions are `MAJOR.MINOR.PATCH`; tags carry no
44
`v` prefix.
55

6+
## Unreleased
7+
8+
### Fixed
9+
10+
- **Writing an h5ad with a `None` in it to Zarr failed.** anndata stores
11+
`None` as a null, which on HDF5 is an empty dataspace with no value to
12+
read, and copying one raised `TypeError: float() argument must be a string
13+
or a real number, not 'Empty'`. Any `None` in `uns` was enough, so
14+
`subset in.h5ad -o out.zarr` failed on such stores, and so did `split`,
15+
`import` and `convert` writing Zarr from them. Nulls are now written in the
16+
target's own spelling: an empty dataspace on HDF5, a 0-d bool on Zarr, as
17+
anndata does. The 0-d bool that a Zarr store uses for a missing `raw` now
18+
becomes an empty dataspace in an h5ad too, rather than a stray `False`.
19+
20+
- **Zarr written to h5ad came out uncompressed.** Zarr codecs have no HDF5
21+
equivalent, so they were dropped and nothing replaced them; a compressed
22+
Zarr store became an h5ad several times its size. A compressed array now
23+
becomes a gzip-compressed dataset (level 4, h5py's default). Strings and
24+
arrays that were not compressed are left as they were.
25+
26+
- **An empty array chunked larger than itself could not be written to
27+
h5ad.** Zarr allows a chunk bigger than its array and h5py does not, so a
28+
zero-length array copied from Zarr failed, and so did `convert --dtype` or
29+
`--layout` on an all-zero sparse matrix from a compressed h5ad. A
30+
zero-length dataset now lets HDF5 choose its own chunks.
31+
632
## 0.6.1
733

834
Makes the outputs of `subset`, `split` and `concat` the size anndata writes

‎docs/ELEMENTS_zarr.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,11 @@ raises `Expected an ArrayArrayCodec`, because the v3 string data type encodes
340340
variable length itself. This tool drops the filter and resolves the dtype for
341341
the target rather than reusing the source's.
342342

343+
Codecs travel only between stores of the same Zarr version. Across backends
344+
none do: an HDF5 dataset written to Zarr gets zarr-python's default codec, and
345+
a compressed Zarr array written to HDF5 becomes a gzip dataset at level 4
346+
(h5py's default). An array that was not compressed stays uncompressed.
347+
343348
Note also that `zarr-python` flags both `NullTerminatedBytes` (what `dtype="S"`
344349
produces) and `FixedLengthUTF32` (what `<U` produces) as having no v3
345350
specification, and warns that other Zarr libraries may not be able to read

‎src/adata/core/convert.py‎

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@
3636
copy_attrs,
3737
create_dataset,
3838
dataset_create_kwargs,
39+
fit_chunks,
3940
is_dataset,
4041
is_group,
4142
is_zarr_group,
@@ -691,14 +692,12 @@ def cast_dense(
691692
kw = dataset_create_kwargs(
692693
src.obj, target_backend=backend, dst_parent=dst_parent
693694
)
694-
from adata.core.subset import _clamp_chunks
695-
696695
dst = create_dataset(
697696
dst_parent,
698697
name,
699698
shape=(n_rows, n_cols),
700699
dtype=data_dtype,
701-
**_clamp_chunks(kw, n_rows, n_cols),
700+
**fit_chunks(kw, (n_rows, n_cols), backend=backend),
702701
)
703702
copy_attrs(src.obj.attrs, dst.attrs, target_backend=backend)
704703

‎src/adata/core/subset.py‎

Lines changed: 4 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
copy_tree,
3535
dataset_create_kwargs,
3636
detect_backend,
37+
fit_chunks,
3738
is_dataset,
3839
is_group,
3940
is_zarr_group,
@@ -161,7 +162,7 @@ def _copy_rows(
161162
kw = dataset_create_kwargs(
162163
src_ds, target_backend=target_backend, dst_parent=dst_parent
163164
)
164-
kw = _clamp_chunks(kw, len(indices))
165+
kw = fit_chunks(kw, (len(indices),), backend=target_backend)
165166
ds = create_dataset(
166167
dst_parent,
167168
name,
@@ -172,35 +173,6 @@ def _copy_rows(
172173
return ds
173174

174175

175-
def _clamp_chunks(kw: dict, *out_shape: int) -> dict:
176-
"""Shrink a forwarded chunk shape to fit the subset's dimensions.
177-
178-
h5py rejects a chunk larger than the dataset, so a chunked source subset
179-
below its own chunk size would otherwise fail outright.
180-
181-
Zarr additionally requires a shard to be a whole number of chunks, so a
182-
clamped chunk invalidates the source's shard geometry. Sharding is a
183-
storage-layout choice rather than data, so it is dropped and left to the
184-
backend rather than recomputed into something arbitrary.
185-
"""
186-
chunks = kw.get("chunks")
187-
if not isinstance(chunks, (tuple, list)) or not chunks:
188-
return kw
189-
190-
original = tuple(int(c) for c in chunks)
191-
clamped = tuple(
192-
min(c, out_shape[i]) if i < len(out_shape) and out_shape[i] > 0 else c
193-
for i, c in enumerate(original)
194-
)
195-
if clamped == original:
196-
return kw
197-
198-
kw = dict(kw)
199-
kw["chunks"] = clamped
200-
kw.pop("shards", None)
201-
return kw
202-
203-
204176
def subset_axis_group(
205177
src: Any,
206178
dst: Any,
@@ -342,7 +314,7 @@ def fan_out_dense_matrix(
342314
kw = dataset_create_kwargs(
343315
src, target_backend=target_backend, dst_parent=dst_parent
344316
)
345-
kw = _clamp_chunks(kw, out_obs, out_var)
317+
kw = fit_chunks(kw, (out_obs, out_var), backend=target_backend)
346318
dst = create_dataset(
347319
dst_parent, name, shape=(out_obs, out_var), dtype=src.dtype, **kw
348320
)
@@ -569,7 +541,7 @@ def _sparse_dataset(
569541
"""
570542
backend = _target_backend(group)
571543
kw = dataset_create_kwargs(template, target_backend=backend, dst_parent=group)
572-
kw = _clamp_chunks(kw, max(1, n))
544+
kw = fit_chunks(kw, (n,), backend=backend)
573545
if "chunks" not in kw and n:
574546
kw["chunks"] = (min(n, 1 << 16),)
575547
return create_dataset(group, name, shape=(n,), dtype=dtype, **kw)

‎src/adata/storage/__init__.py‎

Lines changed: 89 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -284,6 +284,19 @@ def dataset_create_kwargs(
284284
kw["scaleoffset"] = src.scaleoffset
285285
if src.fillvalue is not None:
286286
kw["fillvalue"] = src.fillvalue
287+
if (
288+
target_backend == "hdf5"
289+
and is_zarr_array(src)
290+
and chunks is not None
291+
and tuple(getattr(src, "shape", ())) != ()
292+
and not _is_string_src(src)
293+
and _zarr_is_compressed(src)
294+
):
295+
# Zarr codecs have no HDF5 equivalent, but compression itself does.
296+
# Without this a compressed Zarr store came out of every command
297+
# uncompressed, several times its size.
298+
kw["compression"] = "gzip"
299+
kw["compression_opts"] = GZIP_LEVEL
287300
if target_backend == "zarr" and is_zarr_array(src):
288301
src_zarr_format = getattr(getattr(src, "metadata", None), "zarr_format", None)
289302
target_format = _target_zarr_format(kw_target)
@@ -424,6 +437,68 @@ def create_dataset(
424437
return parent.create_dataset(name, shape=shape, dtype=dtype, **kwargs)
425438

426439

440+
#: gzip level for an HDF5 copy of a compressed Zarr array: h5py's default,
441+
#: and what Cell Ranger and most other HDF5 writers use.
442+
GZIP_LEVEL = 4
443+
444+
445+
def _zarr_is_compressed(src: Any) -> bool:
446+
"""Does this Zarr array carry a compressor? (`compressors` covers v2 too.)"""
447+
try:
448+
return bool(getattr(src, "compressors", None))
449+
except Exception:
450+
return False
451+
452+
453+
def fit_chunks(kw: dict, shape: Sequence[int], *, backend: str) -> dict:
454+
"""Make a forwarded chunk shape legal for a dataset of `shape`.
455+
456+
h5py rejects a chunk larger than the dataset, so a chunked source copied
457+
or subset below its own chunk size would otherwise fail outright. A
458+
zero-length dimension cannot be fitted at all; HDF5 is then left to pick
459+
its own chunking, which it does whenever compression asks for one. Zarr
460+
accepts a chunk larger than its array, so there it only has to shrink.
461+
462+
Zarr additionally requires a shard to be a whole number of chunks, so a
463+
clamped chunk invalidates the source's shard geometry. Sharding is a
464+
storage-layout choice rather than data, so it is dropped and left to the
465+
backend rather than recomputed into something arbitrary.
466+
"""
467+
chunks = kw.get("chunks")
468+
if not isinstance(chunks, (tuple, list)) or not chunks:
469+
return kw
470+
471+
original = tuple(int(c) for c in chunks)
472+
dims = [int(n) for n in shape]
473+
if backend == "hdf5" and any(n == 0 for n in dims[: len(original)]):
474+
kw = dict(kw)
475+
kw.pop("chunks")
476+
kw.pop("shards", None)
477+
return kw
478+
479+
clamped = tuple(
480+
min(c, dims[i]) if i < len(dims) and dims[i] > 0 else c
481+
for i, c in enumerate(original)
482+
)
483+
if clamped == original:
484+
return kw
485+
486+
kw = dict(kw)
487+
kw["chunks"] = clamped
488+
kw.pop("shards", None)
489+
return kw
490+
491+
492+
def _is_null(src: Any) -> bool:
493+
"""Is `src` an explicit null -- `h5py.Empty` on HDF5, or tagged `null`?"""
494+
if is_hdf5_dataset(src) and src.shape is None:
495+
return True
496+
value = src.attrs.get("encoding-type", None)
497+
if isinstance(value, bytes):
498+
value = value.decode("utf-8")
499+
return value == "null"
500+
501+
427502
def _target_zarr_format(zarr_format: Optional[int]) -> Optional[int]:
428503
"""The destination's Zarr version, or None when the caller did not say.
429504
@@ -570,17 +645,27 @@ def copy_dataset(src: Any, dst_group: Any, name: str) -> Any:
570645
"""
571646
from adata.elements.strings import target_dtype
572647

573-
shape = tuple(src.shape) if getattr(src, "shape", None) is not None else ()
574648
target_backend = "zarr" if is_zarr_group(dst_group) else "hdf5"
649+
if _is_null(src):
650+
# anndata writes an absent element, `raw` most often, as a null: an
651+
# empty dataspace on HDF5 and a 0-d bool on Zarr. Neither can be read
652+
# and written back as a value -- `h5py.Empty` has none -- so it is
653+
# written afresh in the target's own spelling.
654+
from adata.elements.write import write_null
655+
656+
ds = write_null(dst_group, name)
657+
copy_attrs(src.attrs, ds.attrs, target_backend=target_backend)
658+
return ds
659+
660+
shape = tuple(src.shape)
575661
zformat = zarr_format_of(dst_group)
662+
kw = dataset_create_kwargs(src, target_backend=target_backend, zarr_format=zformat)
576663
ds = create_dataset(
577664
dst_group,
578665
name,
579666
shape=shape,
580667
dtype=target_dtype(src.dtype, target_backend, zformat),
581-
**dataset_create_kwargs(
582-
src, target_backend=target_backend, zarr_format=zformat
583-
),
668+
**fit_chunks(kw, shape, backend=target_backend),
584669
)
585670
copy_attrs(src.attrs, ds.attrs, target_backend=target_backend)
586671

‎tests/test_anndata_roundtrip.py‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,39 @@ def test_subset_roundtrips_through_anndata(reference, out_fmt, tmp_path):
181181
assert "counts" in got.layers
182182

183183

184+
@pytest.mark.parametrize("out_fmt", ["h5ad", "zarr"])
185+
def test_subset_of_a_store_holding_nulls_crosses_backends(fmt, out_fmt, tmp_path):
186+
"""anndata writes `None` as a null: an empty HDF5 dataspace, or a 0-d bool
187+
on Zarr, where it also stands in for a missing `raw`.
188+
189+
An empty dataspace has no value to read, so copying one to Zarr raised a
190+
TypeError, taking down `subset -o out.zarr` of any h5ad with a `None`
191+
anywhere in `uns`. The reference store has none, which is how it went
192+
unnoticed.
193+
"""
194+
obj = ad.AnnData(
195+
X=sparse.csr_matrix(np.eye(N_OBS, N_VAR, dtype="float32")),
196+
obs=pd.DataFrame(index=[f"cell_{i}" for i in range(N_OBS)]),
197+
var=pd.DataFrame(index=[f"gene_{i}" for i in range(N_VAR)]),
198+
)
199+
obj.uns["params"] = {"layer": None, "k": 5}
200+
source = tmp_path / f"nulls.{fmt}"
201+
obj.write_h5ad(source) if fmt == "h5ad" else obj.write_zarr(source)
202+
203+
names = tmp_path / "keep.txt"
204+
names.write_text("cell_0\ncell_2\n")
205+
out = tmp_path / f"subset.{out_fmt}"
206+
result = runner.invoke(
207+
app, ["subset", str(source), "-o", str(out), "--obs", str(names)]
208+
)
209+
assert result.exit_code == 0, result.stdout + (result.stderr or "")
210+
211+
got = _read(out)
212+
assert got.shape == (2, N_VAR)
213+
assert got.raw is None
214+
assert got.uns["params"] == {"layer": None, "k": 5}
215+
216+
184217
def test_subset_matches_var_on_raws_own_axis(reference, tmp_path):
185218
"""raw/ carries its own var axis and must be matched against it."""
186219
names = tmp_path / "vkeep.txt"

‎tests/test_convert.py‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -208,6 +208,25 @@ def test_indices_can_be_narrowed_when_asked(tmp_path):
208208
assert np.array_equal(ad.read_h5ad(out).X.toarray(), _matrix().toarray())
209209

210210

211+
@pytest.mark.parametrize("args", [["--dtype", "float32"], ["--layout", "csc"]])
212+
def test_an_all_zero_compressed_matrix_converts(tmp_path, args):
213+
"""A compressed store chunks even its empty arrays, and fitting that chunk
214+
to zero elements produced `(1,)`, which h5py rejects for a `(0,)` array."""
215+
obj = ad.AnnData(
216+
X=sparse.csr_matrix((6, 4), dtype="float64"),
217+
obs=pd.DataFrame(index=[f"c{i}" for i in range(6)]),
218+
var=pd.DataFrame(index=[f"g{i}" for i in range(4)]),
219+
)
220+
store = tmp_path / "zeros.h5ad"
221+
obj.write_h5ad(store, compression="gzip")
222+
out = tmp_path / "out.h5ad"
223+
224+
result = runner.invoke(app, ["convert", str(store), "X", "-o", str(out), *args])
225+
assert result.exit_code == 0, _out(result)
226+
got = ad.read_h5ad(out).X
227+
assert got.shape == (6, 4) and got.nnz == 0
228+
229+
211230
# ---------------------------------------------------------------------------
212231
# refusing before writing
213232

0 commit comments

Comments
 (0)