Repository navigation
Conversation
- 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>
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
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.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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
inlists 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 pandasisin, directly and on stores (tests/test_query.py,tests/test_select.py).obs_names.isindoes, on both axes and in raw/var matching. The search no longer stops once every name has been seen.subsetreports how many names matched more than one row. Tested againstadata[obs_names.isin(...), var_names.isin(...)](tests/test_subset_selection.py,tests/test_subset.py).commands/split.py. The root cause was still inutil/names.unique_names, and it reproduced throughconvert --split-feature-types: typesA+,AandA_1failed with a false "another adata run is writing" message.unique_namesnow claims each stem case-insensitively against every stem already given out, so split,--split-feature-typesand Parse--all-samplesall get distinct names. The claims go in a fixed order: missing values takeNAfirst, then labels that are already filenames take their own names, then the remaining labels take their sanitised forms if free. SoCD4,CD4_1andCD4+giveCD4,CD4_1andCD4_2. Split's_distinctworkaround is removed (tests/test_split_names.py).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.tests/test_subset_oracle.pycompares every element of subset outputs (by name list and by query, on both axes) and split outputs (by obs and by var) withadata[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.n_counts>1000andcluster==Aparse. A keyword in a value position is read as a value, sostate in (OR, WA)works. A lone=or!gets "Unknown operator ... use ==, !=, <, <=, > or >=", and an unclosed quote is reported as one. Tested against pandaseval.core/select.read_column). Numbers and booleans compare by value at the column's dtype, as pandas compares them: float32pct_mt <= 0.1keeps float32 0.1,total_counts != 0drops the zeros, andis_doublet == truematches. Missing numbers equal nothing, so!=keeps them, andnanselects 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.group_indiceskeeps missing values (category code -1, masked, NaN) apart from"". split writes them to their ownNAstore, which claims that name first, so a""label getsNA_1. The manifest lists the missing value as empty. Tested against pandas grouping withdropna=False, and through the CLI for categorical and nullable-string columns.subset_axis_groupreads the encoding withspec.encoding_type, which is None when untagged, so an untaggedcodes+categoriesgroup is narrowed with the rest of its frame. Tested against pandas, since anndata reads this layout as NaN.as_viewof a frame), including on an axis kept whole. Tested against anndata for subset on obs, var and both, and for split.subset_h5ad(quiet=...),conversion/text._fields(n_lines), and theadata.infoandadata.readshims;tests/test_info_read.pynow importsadata.core.infoandadata.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' unusedn_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_nameson 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:
ORinstate 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.CD4_1.h5adheld labelCD4. Fixed: labels that are already filenames keep their own names (see F044). Tested; docs corrected.value. Any CSV encoding is ambiguous to pandas, which reads both""andNAas NaN. Docs and docstring corrected: the store name (NAfor missing) tells them apart.### Changed. A quotedinvalue with a comma is now one label. The internalindices_from_name_setnow returns three values.subset_h5ad(quiet=)and theadata.info/adata.readmodules are removed.inlists 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.pyskips without pandas, which CI always has.Changelog entry
Fixed
subset --obs/--varkept only the first row with each name, so after aconcatwithout--index-unique, a barcode list kept only the first sample's cells. It now keeps what anndata'sobs_names.isin(names)keeps, raw included, and says how many names matched more than one row.inlists (#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.total_counts != 0kept the zeros of a float32 column,is_doublet == truematched nothing, and float32 values of 0.1 missedpct_mt <= 0.1. Values now compare at the column's dtype, as pandas compares them. A missing number equals no value, andnanselects it. A value of the wrong type is refused.n_counts>1000failed with "Unexpected end of query."; it now parses. A lone=or!says which operators there are.--inplacethe 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.subsetandsplitnow 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.""went into oneNAstore. Missing values, NaN included, now get their ownNAstore, and the manifest lists their value as empty.split,convert --split-feature-typesand--all-samplescould collide: labelsA+,AandA_1gaveA,A_1andA_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
inlist 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 writecluster in 1,2,3orcluster in ("1", "2", "3")for three labels.CD4,CD4_1andCD4+now giveCD4.h5ad,CD4_1.h5adandCD4_2.h5ad; the label that needed sanitising takes the suffix. Missing values, NaN included, claimNAfirst, so a literalNAlabel moves toNA_1when the column also has missing values, and NaN in a float column goes toNArather thannan.adata.infoandadata.readmodules (#94). They only re-exportedadata.core.infoandadata.core.read.subset_h5adlost its unusedquietargument, andindices_from_name_setnow also returns how many names matched more than one row.🤖 Generated with Claude Code