Repository navigation
Give photo its command line back, and cover the parser that lost it - #1
Merged
Merged
Conversation
`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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
python -m photo.clihas raisedTypeErroron every invocation since G10 was removed —--helpincluded:3695b22("Remove G10, and leave its number vacant") deleted the body of the--bleach-photonsargument and left the call behind:argparseconstructs 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 insidemain(), andmain()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.pycloses that for all ten*/cli.pymodules rather than just this one:--helpmust 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
photowith the empty call restored, and passes with it removed: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 suppliessection quotes this CLI's before/after output, which a reviewer cannot reproduce until this lands.🤖 Generated with Claude Code