Skip to content

Reverse-conversion (h5ad → Seurat) integrity overhaul - #2

Merged
mianaz merged 3 commits into
mainfrom
claude/fix-items-383z97
Aug 21, 2026
Merged

Reverse-conversion (h5ad → Seurat) integrity overhaul#2
mianaz merged 3 commits into
mainfrom
claude/fix-items-383z97

Conversation

@mianaz

@mianaz mianaz commented Aug 21, 2026

Copy link
Copy Markdown
Owner

Hardens readH5AD() and the h5ad writers against the known failure modes of full-object reverse conversion, so the read direction gets the same read-back discipline the write direction already has.

What changed

Counts-layer ambiguity (version-aware resolution). Where raw counts live is now resolved explicitly, in priority order: a new /uns/scConvert/counts_location stamp → /layers/counts (modern convention) → /raw/X (legacy scanpy) → /X. The resolved slot lands in the Seurat counts layer and X in data in every path — in-memory, components subsets, the C-reader fallback, and BPCells on-disk mode (which previously only knew raw/X and silently served log-normalized X as on-disk "counts" for layers['counts'] files). Failing to relocate counts out of layers/counts is now a hard scConvert_data_error instead of a swallowed message.

Counts integrality guard. A counts layer left holding non-integer values raises a classed scConvert_counts_warning naming the slot it came from.

Writer provenance stamp. All three h5ad writers (.writeH5AD_c post-pass, DirectSeuratToH5AD, H5SeuratToH5AD) stamp /uns/scConvert/{version, counts_location} by inspecting the file just written — not the writer's intent — so readers branch on recorded fact instead of layout heuristics. Verified the stamp reads back cleanly in Python anndata as a plain uns dict.

Post-read verification. Before returning, the loaded object is asserted against the file: dims vs the file's obs/var counts (transpose/orientation check), cell names identical to the obs index in order, feature names identical to the var index modulo Seurat's documented underscore→dash replacement, and no duplicate barcodes/genes. Violations raise scConvert_data_error; what the reader did is recorded in misc$scConvert_read.

Duplicate names are loud. Duplicate barcodes/features dedupe with a classed scConvert_names_warning instead of silently.

Gene identity is never lost. When final feature names differ from the file's var index (dedup or underscore mangling), the original identifiers are preserved in an orig_var_index feature-metadata column. Var metadata and layer matrices are now keyed by the object's final rownames — fixing a real pre-existing bug where underscore gene names silently misaligned var columns and dropped layers (including layers/counts itself).

Reduction keys. Colliding obsm keys (e.g. X_pca + pca) no longer silently overwrite each other; later claimants keep their raw key with a scConvert_reduction_warning. New readH5AD(reductions = c(scvi = "X_scVI")) argument selects and renames obsm keys explicitly.

Categoricals. Stored category order stays the factor level order (now pinned by tests against non-alphabetical orders), and the AnnData ordered flag round-trips into ordered factors in both reader paths (DecodeCategorical() gains ordered=; the C path restores the flag post-hoc).

Shared helpers live in the new R/VerifyH5AD.R; the sparse/dense matrix reader moved there unchanged from the readH5AD closure so all paths share one implementation.

Testing

  • New tests/testthat/test-reverse-conversion.R: 80 assertions pinning the contract across both reader paths (hdf5r and C) — counts resolution incl. components subsets and the stamp override, warning classes, verification errors, dedup + orig_var_index, collision/remap behavior, category order/orderedness, and a full write→read round trip.
  • Full suite: 843 pass; the only 6 failures (h5mu, zarr vlen-utf8) reproduce identically on unmodified main in the dev container (HDF5 1.10 there vs the required 1.14+) and are unrelated.
  • R CMD check Rd/usage/code checks clean; a writeH5AD output with the new stamp loads cleanly in Python anndata with layers['counts'] and uns['scConvert'] intact.

Generated by Claude Code

claude added 3 commits August 20, 2026 23:50
…on, key remapping

Reverse-conversion integrity overhaul for the h5ad -> Seurat direction,
addressing the known failure modes of full-object imports:

- Counts-layer ambiguity: where raw counts live is now resolved
  explicitly (uns/scConvert stamp > layers/counts > raw/X > X) and
  honored in every path -- in-memory, components subsets, the C-reader
  fallback, and BPCells on-disk mode (which previously only knew raw/X
  and served log-normalized X as on-disk counts for layers['counts']
  files). Failure to relocate counts out of layers/counts is now a hard
  scConvert_data_error instead of a swallowed message.
- Counts integrality guard: a counts layer left holding non-integer
  values raises a classed scConvert_counts_warning naming its source.
- Post-read verification: dims vs the file's obs/var counts
  (transpose/orientation check), cell-name identity and order vs the obs
  index, feature-name identity modulo Seurat's underscore-to-dash
  replacement, and barcode/gene uniqueness are asserted before the
  object is returned; what the reader did is recorded in
  misc$scConvert_read.
- Duplicate cell barcodes and feature names dedupe loudly
  (scConvert_names_warning) instead of silently.
- Gene identity: original var index preserved as orig_var_index feature
  metadata when names change; var metadata and layer matrices are keyed
  by the object's final rownames, fixing silent misalignment and layer
  drops for underscore gene names.
- Reduction keys: colliding obsm keys no longer silently overwrite each
  other; new readH5AD(reductions=) argument selects/renames obsm keys.
- Categoricals: stored category order stays the factor level order and
  the AnnData ordered flag round-trips (DecodeCategorical gains
  ordered=; the C path restores the flag post-hoc).

Shared helpers live in R/VerifyH5AD.R; the sparse/dense matrix reader
moved there unchanged from the readH5AD closure so all paths share it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3bYjhyFftW5pXgRXDs27
The C writer path, DirectSeuratToH5AD, and H5SeuratToH5AD now write
/uns/scConvert/{version, counts_location} into every h5ad they produce.
The stamp is derived by inspecting the file just written (layers/counts,
raw/X, X, or none) rather than trusting the writer's intent, so readers
can branch on recorded fact instead of layout heuristics -- the
version-aware branch readH5AD() now checks first when resolving the
counts layer.

The reader-side provenance record (misc$scConvert_read) is excluded
from uns on export: it describes a past read, not the file being
written.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3bYjhyFftW5pXgRXDs27
Pins the new readH5AD/writeH5AD contract across both reader paths
(hdf5r and C):

- layers/counts -> counts layer, X -> data (also under components
  subsets), with provenance recorded
- uns/scConvert counts_location stamp overriding layout heuristics
- classed scConvert_counts_warning on non-integer counts, and silence
  for integer-valued X
- .h5ad_verify_read orientation/identity/duplicate assertions
- duplicate feature/barcode dedup warnings, orig_var_index preservation,
  var-metadata alignment under underscore mangling
- obsm key collisions kept under distinct names; reductions= subset and
  rename; .h5ad_plan_reductions unit coverage
- non-alphabetical category order and the ordered flag surviving reads
- writeH5AD stamping and a full write/read round trip restoring integer
  counts and log-normalized data

Also documents the overhaul in NEWS.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01AB3bYjhyFftW5pXgRXDs27
@mianaz
mianaz merged commit fedb411 into main Aug 21, 2026
6 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants