Read AnnData's nullable-encoded columns and v3 categoricals - #101
Conversation
|
Warning Review limit reached
Next review available in: 2 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
fd818ea to
6bcbf01
Compare
382e412 to
6abb630
Compare
6abb630 to
b625f8f
Compare
|
@CodeRabbit review |
✅ Action performedReview finished.
|
A nullable column is stored as a *group* of `values` + `mask`, not an array, so opening it as an array fails outright. Because `obs/_index` and `var/_index` are themselves columns, the visible symptom was missing variable names rather than a missing value — the browser fell back to `varN`. This is not confined to stores our writer has touched: AnnData writes nullable encodings by default from 0.13, and `spatialdata` inherits that, so freshly written stores carry them too. Handles all three encodings sharing the layout — `nullable-string-array`, `nullable-integer`, `nullable-boolean` — since they differ only in the dtype of `values`. The mask is applied rather than discarded, so a missing entry reads as `null` and stays distinguishable from an empty string or a real zero. Both read paths are covered: `_loadColumn`, which dispatches on `encoding-type`, and `getFlatArrDecompressed`, which index reads reach directly and which now resolves the node before assuming it is an array. A group with any other encoding still fails, with a message naming what was found. Not routed through `anndata.js`: it dispatches none of the nullable encodings (in the published 0.0.2 and on main), and it pins zarrita 0.5.1, which is the subject of the still-open upstream #48. Our index and column reads already deliberately bypass it for exactly this reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A categorical column's values were only decoded when its categories array
had dtype `v2:object`. A zarr v3 store writes them as `string`, so the
check failed and the column resolved to its raw integer codes — plausible
looking numbers rather than an error, and wrong wherever a label was
expected. v2 fixed-width unicode (`v2:U*`) had the same problem.
Test against the type rather than one spelling of it: zarrita's `is()`
already knows that `string`, `v2:U*` and `v2:S*` are all text, so the
check becomes `is('string') || is('object')`.
Also map pandas' -1 "missing" code to null instead of indexing off the end
of the categories array and yielding undefined.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`classifyObsColumnNode` settles a column's kind from its `encoding-type` where one is decisive, and otherwise falls back to the dtype in the node's array metadata. A nullable column is a group of `values` + `mask`, so it has no array metadata of its own, and its encoding was not in the lookup — every one of them came back `undefined`, which sends `'auto'` mode back to sniffing decoded values. That is not a rare shape. AnnData 0.13 defaults to zarr v3 and writes string columns as `nullable-string-array` there, so on a freshly written store the columns arriving without a declared kind are most of the text ones — the case the lookup exists to avoid. The three encoding names now live in `nullableArrays` alongside the reader, mapped to the kind of their `values`, because reading them and classifying them have to stay in step and are in different modules. Also switches `getObsColumnKinds` to the `getObsGroup()` helper. It was still reaching for `this.parsed.obs` behind a `typeof === 'object'` test, which admits a `LazyZarrArray` — the narrowing a08fd37 introduced for the two accessors either side of it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
b625f8f to
22285df
Compare
1 participant
Teaches the reader two shapes it did not handle, both of which fail silently — plausible output rather than an error.
Nullable-encoded columns
AnnData stores a nullable column as a group of
values+mask, not an array, so opening its path as an array fails outright. The symptom is usually a missing index rather than a missing value, becauseobs/_indexandvar/_indexare ordinary columns — a table loads withvarNin place of gene names, or with no row ids at all.This is not a legacy shape to tolerate.
anndata0.13 defaults tozarr_write_format = 3and writes string columns this way by default,_indexincluded, so it is simply what a freshly writtenspatialdata0.8 store looks like. I confirmed the pandas dtype makes no difference — a plainobjectindex and apd.array(..., dtype="string")one both come out nullable.All three encodings are read (
nullable-string-array,nullable-integer,nullable-boolean) with the mask honoured, so a masked entry decodes asnull. The missing-value handling from #95 already treatsnullas absent, so it stays distinguishable from a real0or''rather than being rendered as one.Declared kinds for the same columns
getObsColumnKindsdid not recognise them either. It settles the kind fromencoding-typewhere one is decisive and otherwise falls back to dtype in the node's array metadata — which a group does not have — so every nullable column came backundefined, sending'auto'mode back to sniffing decoded values. That is the case the lookup exists to avoid, and on a current store it would have applied to most of the text columns.The three encoding names live in
nullableArrays.tsnext to the reader, mapped to the kind of theirvalues. Listing them at the classifier instead would mean two places that must agree about what AnnData writes, in different modules.zarr v3 categoricals
Separately: a v3 categorical decoded to its raw integer codes. The categories array is written as
stringon v3, and the text check tested for one v2 spelling of that dtype (v2:object), so the column resolved to codes — plausible-looking numbers, no error anywhere. It now asks zarrita whether the dtype is text, which covers v3stringand v2U/Salike. Pandas'-1missing code maps tonullinstead of indexing off the end of the categories array.Confirmed on a real store:
feature_typesreads"Gene Expression"andgenomereads"Unknown"where both previously came back as0.Testing
tableElement.spec.tsfor all three encodings against mock tree nodes.readZarrand assert kinds off the real tree — a mock would only assert the shape the mock chose to write.anndatachanges what it writes.I checked the kind test fails without the fix (
qc_count: undefinedwhere'numeric'is expected) rather than passing for unrelated reasons.Known gap
The
0.5.0/0.6.1/0.7.2integration fixtures all predate nullable encodings — I checked, and every one writesobs/_indexas astring-arrayarray. So the integration net has never seen this shape and stays green through it. Av0.8.0fixture would close that; left for its own PR since it touches fixture generation and CI time.Related: #97 (the guards would not have fixed any of this on their own — the group branch is incomplete independently of the casts).
🤖 Generated with Claude Code