Skip to content

fix(concat_on_disk): retain aligned-mapping keys under join="outer" - #2558

Open
alejandro-publius wants to merge 5 commits into
scverse:mainfrom
alejandro-publius:fix-concat-on-disk-outer-mappings
Open

fix(concat_on_disk): retain aligned-mapping keys under join="outer"#2558
alejandro-publius wants to merge 5 commits into
scverse:mainfrom
alejandro-publius:fix-concat-on-disk-outer-mappings

Conversation

@alejandro-publius

Copy link
Copy Markdown

Problem

concat_on_disk writes the aligned mappings (.obsm/.varm and .layers) over the intersection of their keys, regardless of join. Under join="outer" this silently drops any mapping key present in only some of the inputs — unlike in-memory anndata.concat, which keeps the union and fills the missing entries. When the retained keys additionally have differing shapes, the dense path raises ValueError: Shapes do not align instead of padding.

import anndata as ad, numpy as np, pandas as pd
from scipy import sparse

a = ad.AnnData(sparse.csr_matrix((3, 5)), obs=pd.DataFrame(index=[f"a{i}" for i in range(3)]),
               obsm={"X_emb": np.zeros((3, 2))})
b = ad.AnnData(sparse.csr_matrix((2, 5)), obs=pd.DataFrame(index=[f"b{i}" for i in range(2)]))
a.write_h5ad("a.h5ad"); b.write_h5ad("b.h5ad")

ad.concat([a, b], join="outer").obsm            # {'X_emb': (5, 2)}  ✅
ad.experimental.concat_on_disk(["a.h5ad", "b.h5ad"], "out.h5ad", join="outer")
ad.read_h5ad("out.h5ad").obsm                   # {}   ❌ X_emb silently dropped

Fix

_write_concat_mappings now mirrors the in-memory outer_concat_aligned_mapping:

  • takes the union of keys for join="outer" (intersection for "inner", unchanged);
  • generates outer reindexers so each key is padded along its free (feature) axis to the max width;
  • fills keys missing from some objects with fill_value (promoting integer dtypes to float where the fill is NaN), by materializing just those keys through the same concat_arrays path the in-memory concat uses.

Keys present in every object that need no reindexing keep the existing memory-efficient streaming path, so the common case is unchanged. As a side effect this also fixes the inner-join case where differing .obsm widths previously raised.

Tests

  • test_concat_on_disk_outer_mapping_missing_keys.obsm with dense / sparse / dataframe values, one key missing from an object and shared keys of differing widths.
  • test_concat_on_disk_outer_layers_missing_keys — a .layers key missing from one object with a differing var axis (exercises the reindexed path).

Both assert the on-disk result equals in-memory concat. The full tests/test_concatenate_disk.py suite passes (498 tests).

`concat_on_disk` concatenated the aligned mappings (`.obsm`/`.varm` and
`.layers`) over the intersection of their keys regardless of `join`. Under
`join="outer"` this silently dropped any key present in only some inputs
(and, when retained keys had differing shapes, raised in the dense path),
diverging from in-memory `anndata.concat`, which keeps the union and fills
the gaps.

The concatenation-axis mapping writer now mirrors
`outer_concat_aligned_mapping`: it takes the union of keys for outer joins,
generates outer reindexers to pad each key's free axis, and fills keys
missing from some objects with `fill_value` (promoting integer dtypes where
needed) by materializing just those keys through the same `concat_arrays`
path the in-memory concat uses. Keys present in every object that need no
reindexing keep the memory-efficient streaming path.

Closes scverse#2394
@codecov

codecov Bot commented Jul 15, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 93.75000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 85.86%. Comparing base (c37e213) to head (8f2b03b).
⚠️ Report is 4 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/anndata/experimental/merge.py 93.75% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2558      +/-   ##
==========================================
- Coverage   87.81%   85.86%   -1.96%     
==========================================
  Files          49       49              
  Lines        7799     7833      +34     
==========================================
- Hits         6849     6726     -123     
- Misses        950     1107     +157     
Files with missing lines Coverage Δ
src/anndata/experimental/merge.py 90.23% <93.75%> (-0.76%) ⬇️

... and 8 files with indirect coverage changes

…paths

Use numeric dense/sparse/df obsm with an unshared key and differing widths, so
the test exercises union-of-keys, outer reindexing, and the fill paths for all
three element types while staying writable on the minimum supported h5py (the
previous fixture had a NaN-filled string column that older h5py cannot write).
@alejandro-publius
alejandro-publius force-pushed the fix-concat-on-disk-outer-mappings branch from 530db38 to 04aa7a8 Compare July 15, 2026 09:35
@ilan-gold ilan-gold added this to the 0.14.0 milestone Jul 15, 2026
Comment thread src/anndata/experimental/merge.py Outdated
if reindexers is None:
reindexers = gen_outer_reindexers(elems, ns, new_index=index, axis=axis)
arrays = [
(elem if isinstance(elem, pd.DataFrame) else to_memory(elem))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do these need to be brought into memory?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it to avoid rewriting _write_concat_sequence to handle missing elements?

alejandro-publius and others added 2 commits July 25, 2026 09:59
…ng into memory

Addresses review feedback on scverse#2558. The previous approach routed every
outer-join `.obsm`/`.varm`/`.layers` key that needed filling through
`to_memory()` + in-memory `concat_arrays`, pulling the present objects'
full arrays into RAM and defeating the point of `concat_on_disk`.

Instead, keep the existing streaming writers and teach them about missing
keys directly: `write_concat_dense` contributes a lazy `da.full` block and
`write_concat_sparse` contributes an empty sparse block (via
`_sparse_fill_block`) for each object missing the key, while present
objects stay backed/streamed exactly as they do for `X`. `_write_concat_mappings`
now streams every key through `_write_concat_sequence`, so
`_concat_missing_mapping_key` is gone. Fill shapes come from `ns` (per-object
size along the concat axis) and `_target_free_width` (the reindexed width).

Result is byte-identical to `anndata.concat` (regression tests unchanged and
still pass) but no longer materializes present arrays.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPXJeYVpsPJtcG92Zrc44B
…ls for missing keys

Follow-up to the streaming rewrite. Adds two outer-join regression tests:
- a non-default `fill_value` with keys missing from the leading object, which
  exercises the materialized (non-empty) sparse fill and the fill built for the
  first object.
- a sparse `.layers` key missing under `axis=1`, which drives the fill along the
  layer's column axis.

Marks the width-inference guard in `_target_free_width` as unreachable
(`# pragma: no cover`): a key only reaches it via the union/intersection of the
mappings, so at least one object always carries it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01TPXJeYVpsPJtcG92Zrc44B
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

concat_on_disk with join='outer' doesn't retain all .obsm fields

2 participants