Skip to content

B4: Subset, split and query - #115

Merged
Claptar merged 3 commits into
mainfrom
fix/b4
Nov 2, 2026
Merged

Claptar merged 3 commits into
mainfrom
fix/b4

Conversation

@Claptar

@Claptar Claptar commented Oct 30, 2026

Copy link
Copy Markdown
Contributor

Closes #94. Part of #87. For 0.17.5: merge after 0.17.4 is tagged.

Selections now keep exactly the rows the user named, and every element kind is subset the way anndata subsets it. Each fix has a regression test, checked against anndata's own subsetting or against pandas, on h5ad, zarr2 and zarr3.

Findings

  • F012: in lists are now parsed by token. A quoted value is one label, commas included; bare words are still separated by commas, and a parenthesised list takes commas or spaces. Tested against pandas isin, directly and on stores (tests/test_query.py, tests/test_select.py).
  • F014: name lists keep every row with a listed name, as anndata's obs_names.isin does, on both axes and in raw/var matching. The search no longer stops once every name has been seen. subset reports how many names matched more than one row. Tested against adata[obs_names.isin(...), var_names.isin(...)] (tests/test_subset_selection.py, tests/test_subset.py).
  • F044: B1 had already fixed split's paths, with a workaround in commands/split.py. The root cause was still in util/names.unique_names, and it reproduced through convert --split-feature-types: types A+, A and A_1 failed with a false "another adata run is writing" message. unique_names now claims each stem case-insensitively against every stem already given out, so split, --split-feature-types and Parse --all-samples all get distinct names. The claims go in a fixed order: missing values take NA first, then labels that are already filenames take their own names, then the remaining labels take their sanitised forms if free. So CD4, CD4_1 and CD4+ give CD4, CD4_1 and CD4_2. Split's _distinct workaround is removed (tests/test_split_names.py).
  • F059: obsm and varm arrays of any rank are subset on their first axis, with trailing axes kept whole; a 0-d value is copied. A new pre-write check, check_subsettable, refuses a store holding an element subset cannot narrow (an awkward array, a matrix that is not 2-D), naming it, before the output exists. obsp and varp entries with trailing axes, which anndata allows, are subset on their first two. Tested against anndata's slicing of 3-D and 4-D obsm/varm and 3-D obsp/varp, plus a refusal test for subset and split.
  • F092: new tests/test_subset_oracle.py compares every element of subset outputs (by name list and by query, on both axes) and split outputs (by obs and by var) with adata[obs, var] read back by anndata. That covers X (CSR, CSC, dense), CSR, CSC and dense layers, obsm (2-D, 3-D, sparse, dataframe), varm (2-D, 3-D), obsp, varp and raw (X, var, varm). It runs on h5ad, zarr2 and zarr3. raw is compared under the documented rule that a var selection narrows raw by name; anndata keeps raw's var whole. A mutation that gives layers, obsp and varp the wrong rows at the right shape now fails all 36 of these tests.
  • F119: a bare word ends at an operator character, so n_counts>1000 and cluster==A parse. A keyword in a value position is read as a value, so state in (OR, WA) works. A lone = or ! gets "Unknown operator ... use ==, !=, <, <=, > or >=", and an unclosed quote is reported as one. Tested against pandas eval.
  • F120: columns are read at their stored type (core/select.read_column). Numbers and booleans compare by value at the column's dtype, as pandas compares them: float32 pct_mt <= 0.1 keeps float32 0.1, total_counts != 0 drops the zeros, and is_doublet == true matches. Missing numbers equal nothing, so != keeps them, and nan selects them. A value of the wrong type is refused, where it used to make != keep every row. Tested against pandas on plain, nullable-integer, nullable-boolean, float-with-NaN and numeric-categorical columns.
  • F122: group_indices keeps missing values (category code -1, masked, NaN) apart from "". split writes them to their own NA store, which claims that name first, so a "" label gets NA_1. The manifest lists the missing value as empty. Tested against pandas grouping with dropna=False, and through the CLI for categorical and nullable-string columns.
  • F123: subset_axis_group reads the encoding with spec.encoding_type, which is None when untagged, so an untagged codes + categories group is narrowed with the rest of its frame. Tested against pandas, since anndata reads this layout as NaN.
  • F124: obsm and varm dataframes drop unused categories whenever anndata's subsetting does (anndata's as_view of a frame), including on an axis kept whole. Tested against anndata for subset on obs, var and both, and for split.
  • F162: removed subset_h5ad(quiet=...), conversion/text._fields(n_lines), and the adata.info and adata.read shims; tests/test_info_read.py now imports adata.core.info and adata.core.read. This is dead code, so there is no behavioural test; the suite covers the callers.

Every ref reproduced on main except F044's split path, which B1 had fixed; see above.

Files outside the batch

  • src/adata/conversion/text.py: two lines, dropping _fields' unused n_lines (F162).
  • src/adata/info.py, src/adata/read.py: deleted (F162).
  • docs/troubleshooting.md: the query paragraph described the old text comparison. It now describes the new rules, with entries for the new refusals.
  • docs/reference/testing.md: the new test files are listed.

Tests

  • COLUMNS=80 uv run pytest -m "not integration" --cov=adata --cov-fail-under=90: 2875 passed, 1 skipped; coverage 95.67%
  • ADATA_REQUIRE_VERSION_FIXTURES=1 uv run pytest tests/test_anndata_versions.py -m integration: 204 passed.
  • uv run pytest tests/test_docs_are_accurate.py: 370 passed.

Review

A fresh subagent reviewed the diff, the audit entries and AGENTS.md, and reproduced every point it raised by running code. It found no wrong-row or crash bug. It re-ran the F092 mutation: 36 of 36 oracle tests fail. It fuzzed unique_names on 200k label sets with no case-folded collision. It checked every column layout on all three backends against pandas, plus legacy 0.7 categoricals.

What held up, and what was done:

  • Keywords in value lists: making commas their own tokens turned OR in state in (OR, WA) into a keyword, so it was refused where main accepted it. Fixed: a keyword where a value belongs is a value. Tested.
  • n-D obsp/varp refused: anndata writes, reads and subsets obsp/varp entries with trailing axes. Fixed: they are allowed and subset on their first two axes, and are added to the oracle fixture.
  • Naming: the docs sentence "a suffixed name is never another label's own" was wrong, since CD4_1.h5ad held label CD4. Fixed: labels that are already filenames keep their own names (see F044). Tested; docs corrected.
  • Manifest: the missing value and an empty label are both written as an empty value. Any CSV encoding is ambiguous to pandas, which reads both "" and NA as NaN. Docs and docstring corrected: the store name (NA for missing) tells them apart.
  • Meaning and API changes: these are called out in the changelog entry under ### Changed. A quoted in value with a comma is now one label. The internal indices_from_name_set now returns three values. subset_h5ad(quiet=) and the adata.info / adata.read modules are removed.
  • Nits fixed: one mis-indented block, and three test docstrings.
  • Not changed: numeric in lists cost rows × options (0.12 s per 100k rows for 5,000 options). An integer literal over 308 digits against a float column raises OverflowError, as pandas does. tests/test_query.py skips without pandas, which CI always has.

Changelog entry

Fixed

  • Name lists keep every row with a listed name (#94). subset --obs/--var kept only the first row with each name, so after a concat without --index-unique, a barcode list kept only the first sample's cells. It now keeps what anndata's obs_names.isin(names) keeps, raw included, and says how many names matched more than one row.
  • Quoted labels with commas work in in lists (#94). cell_type in ("CD4-positive, alpha-beta T cell", "B cell") split the first label at its comma, missing the CD4 cells and matching others. A quoted value is now one label; bare words are still separated by commas.
  • Queries compare numbers and booleans by value (#94). Equality compared each value's text, so total_counts != 0 kept the zeros of a float32 column, is_doublet == true matched nothing, and float32 values of 0.1 missed pct_mt <= 0.1. Values now compare at the column's dtype, as pandas compares them. A missing number equals no value, and nan selects it. A value of the wrong type is refused.
  • Queries need no spaces around operators (#94). n_counts>1000 failed with "Unexpected end of query."; it now parses. A lone = or ! says which operators there are.
  • obsm and varm arrays of any rank are subset (#94). An obsm or varm array that was not 2-D was copied whole, so anndata could not read the output, and with --inplace the source became unreadable. They are now subset on their first axis. An element subset cannot narrow, such as an awkward array, is refused before anything is written.
  • obsm and varm dataframes drop unused categories, as anndata does (#94). subset and split now narrow their categorical columns as they narrow obs and var. A categorical column stored without encoding attributes is now narrowed with the rest of its frame instead of being copied whole.
  • split keeps missing labels apart from empty ones (#94). Rows with no value and rows labelled "" went into one NA store. Missing values, NaN included, now get their own NA store, and the manifest lists their value as empty.
  • Output names never share a file (#94). Names for split, convert --split-feature-types and --all-samples could collide: labels A+, A and A_1 gave A, A_1 and A_1, and labels differing only in case shared a file on macOS and Windows. The conversion then failed with a false "another adata run is writing" message. Each name is now distinct, case-insensitively.

Changed

  • A quoted value in an in list is one label (#94). cluster in "1,2,3" used to be split at the commas. A quoted value is now taken as written, so write cluster in 1,2,3 or cluster in ("1", "2", "3") for three labels.
  • split names a label that is a filename already after itself (#94). CD4, CD4_1 and CD4+ now give CD4.h5ad, CD4_1.h5ad and CD4_2.h5ad; the label that needed sanitising takes the suffix. Missing values, NaN included, claim NA first, so a literal NA label moves to NA_1 when the column also has missing values, and NaN in a float column goes to NA rather than nan.
  • Removed the adata.info and adata.read modules (#94). They only re-exported adata.core.info and adata.core.read. subset_h5ad lost its unused quiet argument, and indices_from_name_set now also returns how many names matched more than one row.

🤖 Generated with Claude Code

Claptar and others added 2 commits October 30, 2026 14:21
- Name lists keep every row with a listed name, as anndata's isin does,
  and say how many names matched more than one row (F014).
- Query: quoted values keep their commas in `in` lists (F012); operators
  need no spaces, and a lone = or ! says which operators there are
  (F119); numbers and booleans compare by value at the column's dtype,
  missing numbers equal nothing and `nan` names them (F120).
- obsm/varm arrays of any rank are subset on their first axis, and a
  store holding an element subset cannot narrow (an awkward array) is
  refused before anything is written (F059).
- An untagged categorical group is narrowed with its frame (F123), and
  obsm/varm dataframes drop unused categories as anndata does (F124).
- unique_names claims stems case-insensitively against every stem given
  out, so split, --split-feature-types and --all-samples never put two
  outputs on one path (F044); split's own workaround is gone.
- split gives missing values their own NA store, apart from "" (F122).
- Removed subset_h5ad's unused quiet, _fields' unused n_lines and the
  adata.info / adata.read shims (F162).
- Every element after subset and split is checked against anndata's own
  slicing on h5ad, zarr2 and zarr3 (F092).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…n names

- obsp and varp entries of more than two axes are subset as anndata
  subsets them, not refused.
- A keyword where a value belongs is a value (state in (OR, WA)).
- unique_names gives a label that is a filename already its own name
  first, so CD4_1.h5ad holds the label CD4_1.
- The manifest docstring and docs say how an empty label and a missing
  one are told apart; test docstrings and indentation fixed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings October 30, 2026 14:55
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review ⚠️ Failed 2026-10-30T14:55:11.203156Z
We couldn’t complete this review. Comment @codex review to try again.
24baa84 PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

Copilot AI left a comment

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.

Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota.

An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (py3.13)

2 876 tests  +212   2 872 ✅ +212   9m 58s ⏱️ + 1m 25s
    1 suites ±  0       4 💤 ±  0 
    1 files   ±  0       0 ❌ ±  0 

Results for commit 5b007cd. ± Comparison against base commit 0f69417.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Oct 30, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (py3.12)

2 876 tests  +212   2 872 ✅ +212   11m 39s ⏱️ + 3m 40s
    1 suites ±  0       4 💤 ±  0 
    1 files   ±  0       0 ❌ ±  0 

Results for commit 5b007cd. ± Comparison against base commit 0f69417.

♻️ This comment has been updated with latest results.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Claptar
Claptar merged commit e128b71 into main Nov 2, 2026
9 checks passed
@Claptar Claptar mentioned this pull request Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

B4: Subset, split and query

2 participants