Skip to content

Give photo its command line back, and cover the parser that lost it - #1

Merged
kyu-softmatter merged 1 commit into
mainfrom
fix/photo-cli-empty-add-argument
Sep 16, 2026
Merged

kyu-softmatter merged 1 commit into
mainfrom
fix/photo-cli-empty-add-argument

Conversation

@kyu-softmatter

Copy link
Copy Markdown
Owner

python -m photo.cli has raised TypeError on every invocation since G10 was removed — --help included:

TypeError: _ActionsContainer._get_positional_kwargs() missing 1 required positional argument: 'dest'

3695b22 ("Remove G10, and leave its number vacant") deleted the body of the --bleach-photons argument and left the call behind:

    c.add_argument("--lifetime-ns", type=float, default=None)
    c.add_argument(
    )

argparse constructs every argument while building the parser, so an empty call fails there rather than at parse time. That is why nothing worked, not even asking for help. Removing the two lines is the whole fix — the gate itself was never broken.

Why the suite did not catch it

The 1210 tests all passed, and would have kept passing. They call evaluate() and the check functions directly; the parser is only built inside main(), and main() is only reached from a shell. So the defect was invisible to the suite and immediate to anyone who typed the command.

tests/test_cli_parsers.py closes that for all ten */cli.py modules rather than just this one: --help must exit 0. The packages are discovered by glob, so a new lens's CLI is covered the day it lands instead of the day someone remembers the file.

Checked both ways — it fails on photo with the empty call restored, and passes with it removed:

$ git stash -- photo/cli.py && pytest -q tests/test_cli_parsers.py
FAILED tests/test_cli_parsers.py::test_cli_help_exits_cleanly[photo] - TypeError
1 failed, 10 passed

$ git stash pop && pytest -q tests/test_cli_parsers.py
11 passed

Full suite with the fix: 1210 passed, 11 skipped.

How it was found

Trying to run the photo gate on TRITC, to see what supplying a literature fluorescence lifetime would unblock. The entry for that is a separate pull request and it depends on this one — its ## What it supplies section quotes this CLI's before/after output, which a reviewer cannot reproduce until this lands.

🤖 Generated with Claude Code

`python -m photo.cli` has raised `TypeError` on every invocation since G10 was
removed, `--help` included:

    TypeError: _ActionsContainer._get_positional_kwargs() missing 1 required
               positional argument: 'dest'

`3695b22` ("Remove G10, and leave its number vacant") deleted the body of the
`--bleach-photons` argument and left the call behind:

    c.add_argument("--lifetime-ns", type=float, default=None)
    c.add_argument(
    )

`argparse` constructs every argument while building the parser, so an empty
call fails there rather than at parse time -- which is why nothing worked, not
even asking for help. Removing the two lines is the whole fix.

**The 1210 tests around it all passed, and would have kept passing.** They call
`evaluate()` and the check functions directly, and the parser is only built
inside `main()`, which is only reached from a shell. So the defect was invisible
to the suite and immediate to anyone who typed the command -- the gate itself
was never broken.

`tests/test_cli_parsers.py` closes that gap for all ten `*/cli.py` modules, not
just this one: `--help` must exit 0. The packages are discovered by glob rather
than listed, so a new lens's CLI is covered the day it lands. Checked both ways
-- it fails on `photo` with the empty call restored and passes with it removed.

Found while trying to run the photo gate on TRITC to see what a literature
lifetime would unblock; the entry for that is a separate pull request and it
depends on this one.
@kyu-softmatter
kyu-softmatter merged commit 4d81601 into main Sep 16, 2026
4 checks passed
@kyu-softmatter
kyu-softmatter deleted the fix/photo-cli-empty-add-argument branch September 16, 2026 19:59
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.

1 participant