Skip to content

B10: CLI output and documentation sweep - #122

Merged
Claptar merged 6 commits into
mainfrom
fix/b10
Nov 4, 2026
Merged

Claptar merged 6 commits into
mainfrom
fix/b10

Conversation

@Claptar

@Claptar Claptar commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Closes #103. Part of #87. For 0.17.6.

Output pipes cleanly, help matches behaviour, and the docs match the code after every other batch.

Findings

  • F042 (medium): every store- or user-derived string that reaches Rich now goes through util/markup.literal, which escapes as rich.markup.escape does but prints a trailing backslash once, whatever follows it. That covers member names, attribute values, labels and paths in view, ls and split's plan, and in the messages, status lines and progress descriptions of commands/, conversion/, core/ and formats/. pct_counts[mt] and X_[red]umap print as stored, and [/x] no longer crashes the command.

  • F096: view FILE ENTRY on a missing entry raises, so the CLI prints Error: on stderr and exits 1, as ls does. A new _fail helper in cli.py prints every command's error. It prints a KeyError's message without the quotes str() adds.

  • F151: the stderr console no longer forces a terminal. In a log or a pipe there are no colour, cursor or spinner codes, and a plain message, such as the Error: line, is not wrapped at 80 columns. On a terminal nothing changes.

  • F160: convert --help escapes pyadata-cli\[spatial] and \[pickle]. A test renders every command's help and checks that every bracketed word survives.

  • F161: main(argv) and main_deprecated(argv) pass argv to the app. Given argv, a failure raises SystemExit for the caller to catch. Only the command itself ends the process at once (_leave).

  • F159:

    • The --inplace help now says the source is replaced by a store written beside it first (subset, recast, and every import).
    • --to's help and its refusals list only the formats convert can write (conversion.writable_formats()).
    • convert --help separates the formats it reads and writes from those it only reads, and no longer claims every check runs before the output exists.
    • The README and the index say which formats are written.
    • commands.md gains split's --suffix, create's generated names (zero-padded to the axis length), concat's --output, and flag tables for ls, create, export and import.
    • Stale help fixed: the auto-categorical rule, --include-attrs, and export sparse --chunk.
  • F149: AGENTS.md and developers.md, kept in step, now cover:

    • the one exception to the layering (storage/ uses elements/);
    • the hidden info alias;
    • formats/'s parquet, pickled, pandas_frame and validate modules;
    • the consoles and _fail;
    • escaping, as a rule;
    • the shared --chunk and --no-space-check options;
    • the memory harness;
    • the CI jobs.

    The pyproject comments on anndata and pyarrow are corrected too.

  • F150: STATUS.md loses the two released "Fixed in" bullets. It gains waves 3 and 4 as steps, and the follow-ups from every batch's hand-over notes.

  • F155: testing.md gains a table of the CI jobs, under the names GitHub shows, and rows for the 15 test files missing from its map.

  • F157: troubleshooting.md and pipelines.md now list what --chunk does not bound: obs and var columns, images (export, import and a Visium conversion's tissue images), a pycisTopic pickle, and --in-memory. The 0.17.1 changelog entry no longer says images are not loaded whole.

tests/test_docs_are_accurate.py checks two new kinds of drift:

  • every test file has a row in testing.md's map, and every row names a file that exists;
  • every visible option of every command is named in its section of commands.md. A format's own convert flags may be named on its page under reference/formats/ instead.

The second check found create --zarr-format, ls --plain, export's --chunk, --in-memory, --max-elements and --include-attrs, import dataframe --index-column and concat --output missing from the reference.

Hand-over items

Handled:

  • B8a: convert --help, the convert guide, commands.md and commands/convert.py's docstring said every check runs before the output exists. They now say the input and options are checked first, and what only the data shows (an index out of range) stops the copy, with staging keeping the output from appearing.
  • B2b: export array's help no longer says stdout holds the array in memory (checked: already reworded). B8c's export image --scale help: checked, accurate.
  • Follow-ups from the hand-over notes of B1, B2a, B3b, B5, B6a, B6b, B8a, B8b and B8c are recorded in STATUS.md.

Left, with why:

  • B5: Python warnings still reach stderr with their source line. Printing them as Warning: ... changes behaviour beyond this batch's findings. Installed in the app callback, it would also take warnings away from pytest's capture. It is in STATUS.md.

  • CI: the external-readers job is named "10x h5 against scanpy and PyTables" and compatibility "anndata 0.8-0.13", though the first checks every converter and the second reads 0.7.8 too. Renaming a job renames a required check, so that waits for whoever owns the branch rules. testing.md describes what each job really runs. test_concat_versions.py is an integration test that no CI job runs. Both are in STATUS.md and testing.md.

  • B6b (B6b: Block sizing and Zarr v3 shards #121): a trial merge (git merge-tree) conflicts in two places:

    • the memory paragraph of troubleshooting.md, which both PRs edit;
    • testing.md's CI paragraph, which B6b's latest commit (a separate cost guards job) edits where this PR puts a table of jobs.

    Once B6b: Block sizing and Zarr v3 shards #121 lands, two more things are needed:

    • its test_block_layout.py needs a row in testing.md's map, which the new docs test enforces;
    • the cost guards (py3.12/3.13) job needs a row in the CI tables of testing.md, AGENTS.md and developers.md.

    Whichever PR merges second does all of this.

Changelog entry

Fixed

  • Names with square brackets print as the store spells them (#103). Rich read the [mt] of scanpy's pct_counts[mt] as a style tag. view, ls, split and messages showed pct_counts, a name the next command could not find, and a name, label or attribute holding [/x] stopped the command. Every name, label, path and value from a store or the command line is escaped now.
  • view FILE ENTRY on a missing entry fails (#103). It printed its Error: line on stdout and exited 0, so a script took the message for the entry's description. It goes to stderr with exit code 1 now, as ls does.
  • Logs hold plain lines (#103). Written to a file or a pipe, stderr held colour and cursor codes and wrapped messages at 80 columns, so grep '^Error:' adata.log found nothing. Off a terminal there are no codes now and an error is one line. On a terminal nothing changes.
  • convert --help names the extras to install (#103): pyadata-cli[spatial] and pyadata-cli[pickle] showed as pyadata-cli.
  • Help says what the commands do (#103):
    • --to lists only the formats convert can write;
    • --inplace says the source is replaced by a store written beside it, not modified directly;
    • convert --help says which formats are only read.
  • adata.cli.main(argv) runs argv (#103). It parsed the process's own command line instead.
  • The documentation matches the code (#103):
    • the command reference has every option;
    • troubleshooting and the pipelines guide list everything held whole in memory, images among them;
    • the testing page maps every test file and CI job;
    • the 0.17.1 entry above no longer says images are not loaded whole.

Independent review

A fresh subagent with none of this session's context reviewed the diff against the audit entries and AGENTS.md. It ran about 40 commands with both streams redirected, on stores whose names, labels, attribute values and paths held brackets, [/x] and trailing backslashes. It found no Rich MarkupError and no escape codes off a terminal, and every error line stayed one line. It also confirmed colour on a pseudo-terminal. Its findings, and what was done:

  1. Medium, fixed. split's header put a literal ] after the escaped column (obs\[{column}]), so a column x[red closed into a [red] tag. It now escapes the whole obs[column], and AGENTS.md and developers.md say never to write brackets around a value in a template.

  2. Low-medium, fixed. rich.markup.escape doubles a trailing backslash, so a name ending in \ printed \\ wherever text followed it (view, concat's skip warnings, Wrote paths, import messages). The new util/markup.literal replaces every escape call. A parametrised test renders it before a tag and before text.

  3. Low-medium, fixed. main(argv) still ended an embedding process with os._exit on failure. It raises now; _leave applies only to the command.

  4. Low, fixed in the docs. Usage errors (exit 2) are Typer's boxed message, not an Error: line. pipelines.md and troubleshooting.md now confine the one-line promise to exit code 1. Changing Typer's error rendering would change help rendering too.

  5. Low, fixed. testing.md named the wrong jobs as misnamed, and overstated the rank_genes_groups coverage.

  6. Low, fixed. In AGENTS.md:

    • the click note sat inside a list item;
    • "no linter" contradicted the workflow lint job;
    • the fresh-install bullet left out the docs run without click.

    developers.md now mentions main(argv), validate and pandas_frame as AGENTS.md does.

  7. Low, fixed. The lists of what is held whole now include --in-memory in troubleshooting.md, and JSON (import dict parses whole, export dict builds whole) in both. pipelines.md's --chunk table gains export sparse.

Tests

  • New: tests/test_cli_output.py (F042, F096, F151, F160, F161, and the review's findings 1 to 3). Each failed before its fix.
  • New checks in tests/test_docs_are_accurate.py.
  • tests/test_cli.py::test_view_entry_not_found expected exit 0; it now expects exit 1 and the message on stderr.
  • COLUMNS=80 uv run pytest -m "not integration" --cov=adata --cov-fail-under=90: 3902 passed, 7 skipped, 4 xfailed; coverage 95.79% (head 9d488c2).
  • One earlier full run had one failure, test_write_safety.py::test_export_writes_to_devices_and_pipes_as_they_are. Its FIFO reader stayed blocked in read() with no writer left, and since the reader thread is not a daemon, pytest could not exit. The test passed 15 of 15 runs alone and 3 of 3 with all of test_write_safety.py, and nothing in this PR touches that path. It is recorded in STATUS.md as a follow-up.

🤖 Generated with Claude Code

Claptar and others added 2 commits November 3, 2026 15:37
…rgv; docs sweep

Names, labels, paths and attribute values are escaped before Rich prints
them, so view, ls, split and every message show them as the store spells
them (F042); convert --help keeps pyadata-cli[spatial] and [pickle] (F160).
view on a missing entry fails on stderr with exit 1 (F096). The stderr
console no longer forces a terminal, so a log holds no colour or cursor
codes and an error stays on one line (F151). main(argv) and
main_deprecated(argv) run the arguments given (F161). Help and reference
contradictions fixed (F159); AGENTS.md, developers.md, testing.md,
troubleshooting.md, pipelines.md and STATUS.md brought up to date (F149,
F150, F155, F157). The docs test now also checks that every test file is
in testing.md's map and every option in commands.md.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copilot AI balanced review requested due to automatic review settings November 3, 2026 17:50

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. Try requesting a review at a lower effort level.

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.

@chatgpt-codex-connector

chatgpt-codex-connector Bot commented Nov 3, 2026 •

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 ✅ Completed 2026-11-03T17:52:55.804595Z fd41e74 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.

…, docs

- util/markup.literal replaces rich.markup.escape everywhere: escape doubles
  a trailing backslash, which printed twice wherever text followed.
- split's header escapes the whole obs[column]: a column with an unclosed
  bracket closed into a tag on the template's own ].
- main(argv) raises SystemExit for its caller; only the command itself ends
  the process at once on failure.
- Docs: usage errors are Typer's box, not an Error: line; the held-whole
  lists gain JSON and --in-memory; export sparse's --chunk row; testing.md's
  job names and the rank_genes_groups row; AGENTS.md's CI list and the
  click note; developers.md in step.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (py3.13)

3 811 tests  +242   3 799 ✅ +242   8m 29s ⏱️ - 1m 27s
    1 suites ±  0      12 💤 ±  0 
    1 files   ±  0       0 ❌ ±  0 

Results for commit 2e496ff. ± Comparison against base commit 29d1b76.

♻️ This comment has been updated with latest results.

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Test Results (py3.12)

3 811 tests  +242   3 799 ✅ +242   10m 40s ⏱️ + 4m 57s
    1 suites ±  0      12 💤 ±  0 
    1 files   ±  0       0 ❌ ±  0 

Results for commit 2e496ff. ± Comparison against base commit 29d1b76.

♻️ This comment has been updated with latest results.

Claptar and others added 3 commits November 3, 2026 18:10
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
testing.md: B10's job table with B6b's cost-guards job, the unit job's
marker, and a row for test_block_layout.py. troubleshooting.md: B10's
list of what is held whole with B6b's shard-writing bound. AGENTS.md and
developers.md name the cost-guards job.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
typer 0.21 made Rich markup the default for help text; 0.20, the floor,
printed it raw. So on the floors job convert --help showed
'pyadata-cli\[spatial]' with its escape, and the F160 test failed there.
Setting rich_markup_mode on the app and both sub-apps gives the same help
on every typer the floor allows (checked on 0.20.0, 0.21.1 and 0.27.3).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@Claptar
Claptar merged commit c276ee6 into main Nov 4, 2026
13 checks passed
@Claptar Claptar mentioned this pull request Nov 4, 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.

B10: CLI output and documentation sweep

2 participants