Move CLI dependencies to optional cli extra

[Copilot]

Checklist

• Test coverage is present for new code
• The CHANGELOG.md has been updated if necessary
• Mark deprecated code with deprecation warnings
• Dependency spec updated if necessary

Summary

click and rich were installed for API-only users because the CLI imported them as baseline dependencies. This change moves them behind a cli extra and makes spp exit with an install hint when the extra is missing.

• Packaging
  • Move click and rich from base dependencies to [project.optional-dependencies].cli. They stay in the test extra so CLI tests run in CI.
  • Point the spp console script (pyproject.toml and meta.yaml) at space_packet_parser._spp_entry:main. The conda recipe keeps click/rich as run deps because conda has no extras, and check_metadata.py accounts for the cli extra.
• CLI loading / failure mode
  • space_packet_parser.cli is unchanged except that the click/rich imports are guarded and raise MissingExtraError, a new ImportError subclass in exceptions.py, naming the extra. Same pattern as xarr.py.
  • _spp_entry.main() imports the CLI lazily. It catches only MissingExtraError, prints the hint to stderr and exits 1. Any other import failure propagates with its normal traceback. PEP 621 cannot condition a console script on an extra, so the script is always installed.
  • The build workflow installs the built wheel into a fresh venv, asserts the bare install exits non-zero with the hint, then installs [cli] and runs spp --version. The venv matters because the job environment already has rich via twine.
• Behavioral changes folded in
  • parse --packet N rejects an out-of-range or negative index as a usage error (exit 2). Previously N equal to the packet count raised IndexError, and other out-of-range values printed a message and exited 0.
  • parse gains --root-container, matching describe-xtce, so definitions whose root is not named CCSDSPacket can be parsed from the CLI.
  • describe-xtce and parse fail as a usage error (exit 2) when the named root container is not in the document, listing the containers with no BaseContainer as possible roots. Previously both raised a bare KeyError. The CTIM test definition (root CCSDSTelemetryPacket) exercises this.
• Docs / release notes
  • Install docs (getting_started.md, user_guide/xtce_validation.md, README) call out pip install "space_packet_parser[cli]".
  • Changelog entries under Changed (marked _BREAKING_) and Fixed.

pip install space_packet_parser
spp --version
# The `spp` CLI requires the `cli` extra. Install it with `pip install space_packet_parser[cli]`.
# exit code 1

pip install "space_packet_parser[cli]"
spp --version

spp describe-xtce tests/test_data/ctim/ctim_xtce_v1.xml
# Error: Invalid value for --root-container: No SequenceContainer named 'CCSDSPacket' in ...
# Containers with no BaseContainer (possible roots): CCSDSTelemetryPacket
# exit code 2
spp describe-xtce --root-container CCSDSTelemetryPacket tests/test_data/ctim/ctim_xtce_v1.xml

• Fixes Move click/rich to a "cli" extra and fail gracefully when it's missing #274

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 94.86%. Comparing base (ae1338f) to head (d9d6203).

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #275      +/-   ##
==========================================
+ Coverage   94.62%   94.86%   +0.23%     
==========================================
  Files          49       50       +1     
  Lines        4203     4321     +118     
==========================================
+ Hits         3977     4099     +122     
+ Misses        226      222       -4     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[medley56]

Review of the Copilot agent's head (7ba62db), rebased onto main, with fixes in 22bf4da. Tally: 2 critical, 3 warnings, 3 suggestions, 1 nitpick group. Everything below is addressed in 22bf4da.

🛑 CRITICAL — check_metadata.py failed on this branch

meta.yaml run: kept click/rich while pyproject core dropped them, and test.requires lacked the click/rich the pyproject test extra gained. The pre-commit hook runs this script on every commit. Conda has no extras, so the recipe keeps click/rich as run deps the way it already does for xarray; verify_run_dependencies now unions the cli extra, and test.requires mirrors the test extra.

🛑 CRITICAL — Build workflow's spp --version fails on a bare wheel install

_build.yml installs dist/*.whl without the extra and then runs spp --version, which now exits 1. The Build job was skipped on the PR head so CI did not show it. The step now asserts the bare install exits non-zero with the install hint, then installs [cli] and runs spp --version. Reproduced locally against a built wheel in a fresh venv.

⚠️ WARNING — wrapper design did more than the goal needed

_cli_impl.py, _SppCommandProxy, module __getattr__ and __dir__ existed to keep the entry-point string space_packet_parser.cli:spp, which is not public API, and the proxy was not a faithful click.Group (isinstance false, extra kwargs dropped). Replaced with the pattern already in xarr.py: cli.py is the real Click module with a guarded import block raising an ImportError that names the extra, and _spp_entry.main() is the console-script target that prints that error and exits 1. import space_packet_parser.cli without the extra now raises, which is what #274 asked for.

⚠️ WARNING — cli.py failed ruff format --check. Moot after the rewrite.

⚠️ WARNING — rebase conflicts. docs/source/users.md was deleted on main (#273); the two install snippets moved to getting_started.md and user_guide/xtce_validation.md. The changelog entry moved under [Unreleased] above 6.2.0.

💡 SUGGESTION — changelog entry lacked the issue link, and the --packet bound change had no entry. Both added.

💡 SUGGESTION — out-of-range --packet printed a message and exited 0, and the new test locked that in. Now click.BadParameter (exit 2); negative indices are rejected too.

💡 SUGGESTION — tests: sys.modules.pop(...) was never restored, one test name did not match its assertion, and four tests targeted the wrapper's main(). Rewritten against _spp_entry.main() with monkeypatch.delitem. The non-standalone test was dropped; it only exercised Click's own standalone_mode passthrough.

✏️ NITPICK — repo instructions listed Click/Rich as core dependencies; now marked as the cli extra. The shebang on the private module and its autoapi page go away with the rewrite. click/rich stay duplicated in the test extra, matching xarray/numpy.

Verified and unchanged: click/rich are out of core deps and in the cli extra, spp is still declared, nothing else in the package imports click/rich, no lockfile is tracked, the docs build does not need click, README wording is accurate. Full suite: 491 passed at 22bf4da.

🤖 AI-assisted comment, reviewed and approved by @medley56 before posting.

[Copilot]

🟡 Changes recommended

The new build-workflow assertion for “no cli extra installed” is not isolated from other job-installed tooling and may be flaky or ineffective unless run in an isolated environment (or with click/rich explicitly absent).

Get a fresh assessment by requesting another Copilot review.

Pull request overview

This PR implements issue #274 by moving CLI-only dependencies (click, rich) behind an optional cli extra while keeping the spp console script always installed, and ensuring a bare install fails with a clear install hint rather than a traceback.

Changes:

• Packaging: move click/rich to [project.optional-dependencies].cli, keep them in test, and point spp at a new lazy entrypoint module (space_packet_parser._spp_entry:main).
• CLI behavior: guard click/rich imports in space_packet_parser.cli with a friendly ImportError, and tighten parse --packet N bounds to fail as a usage error (exit 2).
• Project hygiene: add integration tests for the new entrypoint/failure mode, update docs/README/CHANGELOG, and update metadata verification for conda’s lack of extras.

File summaries

File	Description
tests/integration/test_cli.py	Adds tests for the new spp entrypoint behavior and --packet bounds handling, plus simulates missing cli deps.
space_packet_parser/cli.py	Guards click/rich imports with an install-hint ImportError; makes --packet out-of-range a Click usage error.
space_packet_parser/_spp_entry.py	New lazy console-script entrypoint that prints the install hint and exits non-zero when cli extra is missing.
scripts/check_metadata.py	Updates metadata checks to union cli + xarray extras into conda run requirements.
README.md	Documents installing the optional CLI via space_packet_parser[cli].
pyproject.toml	Moves click/rich into cli extra; updates spp script entrypoint; keeps deps in test extra.
meta.yaml	Updates conda spp entrypoint; ensures test requirements include click/rich.
docs/source/user_guide/xtce_validation.md	Adds CLI extra install instructions before using spp.
docs/source/getting_started.md	Adds CLI extra install instructions.
CHANGELOG.md	Adds breaking-change notes and documents the --packet exit-code behavior change.
.github/workflows/_build.yml	Adds a wheel-install verification that expects spp --version to fail without the cli extra, then succeed with it.
.github/instructions/space_packet_parser.instructions.md	Updates repo guidance to mark Click/Rich as optional via the cli extra.

Review details

• Files reviewed: 12/12 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[medley56]

This speaks to a couple of possibilities:

1. We are over-testing and could just verify the behavior and leave this particular behavior un-covered.
2. We need to isolate the test from the environment better (more careful mocking)

The thing I don't want to do is create a complex, environment level harnessing approach just to test the handling of a missing dependency.

[medley56]

Fixed in e1c1f44. The "Verify installation of built wheel" step now installs the wheel into a fresh python -m venv and runs spp from there, so the check depends only on the wheel.

One correction to the framing: the check was deterministic, not flaky, and it failed safe. twine does pull in rich, but not click, and import click is the first line of the guarded block, so the bare install failed as intended. A transitive click would have turned the step red rather than letting it pass silently. The venv removes that dependence on import order and on twine's dependency tree.

🤖 AI-assisted comment, reviewed and approved by @medley56 before posting.

[medley56]

Yeah, this seems pretty reasonable I think.

[medley56]

Fixed in e1480a7. cli.py now raises MissingExtraError, a new ImportError subclass in exceptions.py, from the guarded click/rich import, and _spp_entry.main() catches only that class. Any other import failure inside the CLI propagates with its normal traceback. test_spp_entry_point_propagates_unrelated_import_error covers the negative case by blocking an unrelated internal import.

🤖 AI-assisted comment, reviewed and approved by @medley56 before posting.