refactor(cli): rename scripts→cli, add jsonargparse with --config YAML support#413
Open
Borda wants to merge 14 commits into
Open
refactor(cli): rename scripts→cli, add jsonargparse with --config YAML support#413Borda wants to merge 14 commits into
jsonargparse with --config YAML support#413Borda wants to merge 14 commits into
Conversation
- Rename `trackers/scripts/` → `trackers/cli/` and update `pyproject.toml` entry point to `trackers.cli.__main__:main` - Replace `argparse.ArgumentParser` with `jsonargparse.ArgumentParser` in `__main__.py`; all subcommand parsers now use `jsonargparse.DefaultHelpFormatter` - Add `--config` argument with `action="config"` — any subcommand run can now be specified as a YAML file (e.g. `trackers track --config run.yaml`) - Update handler type annotations from `argparse.Namespace` to `jsonargparse.Namespace`; `_add_tracker_params` / `add_*_subparser` use `Any` to accept both argparse and jsonargparse subparser objects - Add `jsonargparse>=4.48.0` to core dependencies; only transitive dep is PyYAML (depth 1, zero own deps) - Migrate all tests from `tests/scripts/` → `tests/cli/`; update patch paths (`trackers.scripts.*` → `trackers.cli.*`) --- Co-authored-by: Claude Code <noreply@anthropic.com>
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors the trackers command-line interface by moving it from trackers/scripts into a dedicated trackers/cli package and switching the parsing backend from argparse to jsonargparse, with the goal of enabling config-file driven runs.
Changes:
- Migrated CLI modules from
trackers/scripts/totrackers/cli/and updated the console entry point. - Replaced
argparseusage withjsonargparseacross CLI commands and introduced a--configoption. - Updated CLI tests to import/patch the new
trackers.cli.*module paths.
Reviewed changes
Copilot reviewed 11 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| uv.lock | Adds resolved dependency entries for jsonargparse and updates the locked dependency set. |
| pyproject.toml | Adds jsonargparse to core deps and updates console script entry point to trackers.cli.__main__:main. |
| src/trackers/cli/init.py | Introduces the new CLI package. |
| src/trackers/cli/main.py | New CLI entry point using jsonargparse, adds --config, registers subcommands. |
| src/trackers/cli/track.py | Migrates track subcommand to jsonargparse and adjusts dynamic tracker-param registration. |
| src/trackers/cli/eval.py | Migrates eval subcommand to jsonargparse. |
| src/trackers/cli/tune.py | Migrates tune subcommand to jsonargparse. |
| src/trackers/cli/download.py | Migrates download subcommand to jsonargparse. |
| src/trackers/cli/progress.py | Adds progress/source-classification utilities under the new CLI package. |
| tests/cli/init.py | Adds the tests/cli package marker. |
| tests/cli/test_track.py | Updates imports to trackers.cli.track. |
| tests/cli/test_eval.py | (Not shown in diff excerpt) If present in PR, validates eval CLI behavior against new module path. |
| tests/cli/test_tune.py | Updates imports/patch paths to trackers.cli.tune. |
| tests/cli/test_download.py | Updates imports/patch paths to trackers.cli.download. |
| tests/cli/test_progress.py | Updates imports/patch paths to trackers.cli.progress. |
Comments suppressed due to low confidence (3)
src/trackers/cli/track.py:262
_add_tracker_paramsnow wrapsgroup.add_argument(...)insuppress(Exception), which will silently hide any error (e.g. invalidtype, bad kwargs, API changes) and can result in tracker parameters not being exposed on the CLI with no signal. This previously only ignored duplicate-argument errors. Please narrow the suppression to the specific expected exception raised for duplicate options (and consider logging/debug output when a param is skipped).
src/trackers/cli/main.py:39--configis added only to the top-level parser. With subcommands, options defined on the parent parser are typically only accepted before the subcommand (e.g.trackers --config run.yaml track ...), while the PR description claimstrackers track --config run.yamlshould work. If the intent is to allow--configafter the subcommand, it likely needs to be registered on each subparser (or provided viaparents=[...]when creating subparsers) / otherwise document the supported ordering explicitly.
src/trackers/cli/main.py:39- The new config-file feature (
--config,action="config") is a key behavior change but there are no CLI tests covering parsing/precedence from a YAML/JSON config (e.g. verifying config values are applied and CLI args override config). Adding a focused test would help prevent regressions injsonargparseintegration and confirm the documented invocation works.
- replace add_subparsers boilerplate with CLI({"track": ..., "eval": ..., "tune": ..., "download": ...}) on typed entry points; subcommands and --config YAML are derived automatically from function signatures
- track.py: collapse 735→547 lines by dropping argparse plumbing; group dynamic per-tracker overrides under a TrackerParams dataclass (--tracker_params.<name>=...) so jsonargparse renders one nested help group
- __main__.py: 76→40 lines; main() now warns, dispatches via CLI, and returns the entry-point exit code
- download.py: positional `dataset` becomes `--dataset` (jsonargparse has no positionals); `--list` renamed to `--list_available`
- update tests/cli/test_download.py and test_tune.py to call the new typed functions directly and exercise the CLI via jsonargparse.CLI(...args=[...]) instead of constructing argparse subparsers
- 498 non-integration tests pass; all five `--help` screens render
---
Co-authored-by: Claude Code <noreply@anthropic.com>
jsonargparse with --config YAML support
- add DetectionOptions, FilteringOptions, OutputOptions, VisualizationOptions, ShowOptions dataclasses; each renders as a named group in --help via jsonargparse[signatures] - track() signature collapses 19 flat args into 6 typed group params (detection, filters, out, vis, show) + flat source and tracker; body unpacks to local vars so internals unchanged - switch jsonargparse>=4.48.0 → jsonargparse[signatures]>=4.48.0 to pull in docstring-parser (group titles and per-field help) - flag renames: --model → --detection.model, --confidence → --detection.confidence, --output → --out.output, --show_boxes → --show.boxes, etc. --- Co-authored-by: Claude Code <noreply@anthropic.com>
- add _BoolFlagParser subclass that swaps type=bool args to ActionYesNo(yes_prefix="", no_prefix="no-"); all plain bool fields now render as --show.ids/--no-show.ids instead of --show.ids {true,false}
- pass parser_class=_BoolFlagParser to CLI(); bool|None fields (e.g. TrackerParams.enable_cmc) unaffected by the type is bool guard
---
Co-authored-by: Claude Code <noreply@anthropic.com>
- Migrate flag names: --model→--detection.model, --classes→--filters.classes, --display→--vis.display, --show-*→--show.*, --mot-output→--out.mot_output, --output→--out.output - Fix list syntax: --metrics CLEAR HOTA Identity→--metrics '[CLEAR,HOTA,Identity]', --columns MOTA HOTA IDF1→--columns '[MOTA,HOTA,IDF1]' (space-separated fails with jsonargparse union validation) - Fix download flags: positional dataset→--dataset, --list→--list_available, --cache-dir→--cache_dir - Fix hyphen→underscore: --gt-dir→--gt_dir, --tracker-dir→--tracker_dir, --detections-dir→--detections_dir, --n-trials→--n_trials - Validated against trackers track/eval/tune/download --help output --- Co-authored-by: Claude Code <noreply@anthropic.com>
- Rename field and all internal usages in track.py (11 sites) - Update --out.mot_results in evaluate.md and detection-quality.md docs --- Co-authored-by: Claude Code <noreply@anthropic.com>
- Merge 13 commits from develop: IoU variants, external CMC module, ByteTrack confidence fix, Tuner seeding/fixed_params/images_dir support - Port tune CLI: add fixed_params, images_dir, enqueue_defaults, seed to cli/tune.py and tests/cli/test_tune.py - Expose iou_variant in TrackerParams; _init_tracker converts to BaseIoU instance; all trackers now accept iou kwarg - Add --tracker_params.iou_variant row to docs/learn/track.md - Add --seed row + fixed_params CLI example to docs/learn/tune.md - Remove deleted scripts/tune.py (conflict: modify/delete resolved) --- Co-authored-by: Claude Code <noreply@anthropic.com>
Add iou_variant string field to TrackerParams; _init_tracker converts it to a BaseIoU instance (IoU/GIoU/DIoU/CIoU/BIoU) before forwarding to any tracker that accepts the iou kwarg. Document in track.md. --- Co-authored-by: Claude Code <noreply@anthropic.com>
- Add `_VARIANTS` dict and `variant_from_name()` factory to `iou.py` — single source of truth; new variants only need one-file change - Remove `_IOU_VARIANTS` dict and 6 IoU class imports from `cli/track.py`; replace with `from trackers.utils.iou import variant_from_name` - Warn (`UserWarning`) in `_init_tracker` when `iou_variant` is supplied but the chosen tracker has no `iou` param, instead of silently dropping it --- Co-authored-by: Claude Code <noreply@anthropic.com>
Copilot review #3281138854: jsonargparse[signatures] pulls in docstring-parser and typeshed-client. No code in trackers/cli/ imports from these packages; plain jsonargparse>=4.48.0 suffices. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Copilot review #3281138773: unknown tracker id or iou_variant name raised unhandled ValueError through jsonargparse.CLI, surfacing as a traceback instead of a clean CLI error. Wrap the _init_tracker call in track() and print to stderr before returning 1. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Copilot review #3281138823: dataclass defaults created once at import time are shared across calls. Change option-group params to None defaults; instantiate inside the function body so each call gets a fresh instance. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Copilot review #3281138907: variant_from_name() is new CLI-facing logic but had no unit test coverage. Add TestVariantFromName class with tests for valid lookups, case-insensitivity, and invalid-name ValueError with repr(name) in the error message. --- Co-authored-by: Claude Code <noreply@anthropic.com>
Copilot review #3281138882 (reclassified [req]): --config is a key behaviour change with no test coverage. Add TestConfigFileSupport covering: YAML values applied, CLI args override config, nested dataclass fields parsed from config. --- Co-authored-by: Claude Code <noreply@anthropic.com>
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.
This pull request migrates the CLI implementation from
argparsetojsonargparse, restructures the CLI code into a newtrackers/clipackage, and updates all related imports and tests accordingly. The changes improve configuration flexibility (including config file support), streamline argument parsing, and modernize the CLI codebase.Migration to
jsonargparseand CLI Refactor:Switched from
argparsetojsonargparsein all CLI modules, enabling advanced features such as config file parsing and improved help formatting. All CLI entrypoints and subcommands now usejsonargparse.ArgumentParserand related types. [1] [2] [3] [4] [5] [6] [7] [8] [9] [10] [11] [12] [13] [14] [15] [16]Added support for loading CLI arguments from YAML/JSON config files via a new
--configoption.Codebase Restructuring:
Moved all CLI-related modules from
trackers/scripts/to a newtrackers/cli/package and updated all internal and test imports to reflect the new structure. [1] [2] [3] [4] [5] [6]Updated the main entry point in
pyproject.tomlto point totrackers.cli.__main__:maininstead of the old scripts location.Test Suite Updates:
trackers.clipackage path. [1] [2] [3] [4] [5] [6] [7] [8] [9]Dependency Updates:
jsonargparse>=4.48.0as a required dependency to support the new CLI implementation.Other:
tests/cli/__init__.py.trackers/scripts/→trackers/cli/and updatepyproject.tomlentry point totrackers.cli.__main__:mainargparse.ArgumentParserwithjsonargparse.ArgumentParserin__main__.py; all subcommand parsers now usejsonargparse.DefaultHelpFormatter--configargument withaction="config"— any subcommand run can now be specified as a YAML file (e.g.trackers track --config run.yaml)argparse.Namespacetojsonargparse.Namespace;_add_tracker_params/add_*_subparseruseAnyto accept both argparse and jsonargparse subparser objectsjsonargparse>=4.48.0to core dependencies; only transitive dep is PyYAML (depth 1, zero own deps)tests/scripts/→tests/cli/; update patch paths (trackers.scripts.*→trackers.cli.*)resolves #406