examples/traffic_analysis: improve CLI argument handling#2059
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request refactors the CLI argument handling in traffic analysis examples by replacing argparse with a flexible approach that uses jsonargparse when available and falls back to positional arguments otherwise. The main execution logic is extracted into dedicated main() functions for improved modularity.
Key changes:
- Replaced
argparsewith conditionaljsonargparseusage, falling back to positional arguments when unavailable - Extracted script logic into
main()functions with proper type hints and docstrings - Updated imports to use
sysinstead ofargparse
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 10 comments.
| File | Description |
|---|---|
| examples/traffic_analysis/ultralytics_example.py | Refactored CLI to use jsonargparse with positional fallback; extracted main() function for YOLO-based traffic analysis |
| examples/traffic_analysis/inference_example.py | Refactored CLI to use jsonargparse with positional fallback; extracted main() function for Inference-based traffic analysis |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| def main( | ||
| source_weights_path: str, | ||
| source_video_path: str, | ||
| target_video_path: str, |
There was a problem hiding this comment.
The target_video_path parameter should be optional with a default value of None to maintain backward compatibility with the previous argparse implementation. The original argparse version had default=None for this parameter. Without making it optional, this is a breaking API change.
| api_key = args.roboflow_api_key | ||
| def main( | ||
| source_video_path: str, | ||
| target_video_path: str, |
There was a problem hiding this comment.
The target_video_path parameter should be optional with a default value of None to maintain backward compatibility with the previous argparse implementation. The original argparse version had default=None for this parameter. Without making it optional, this is a breaking API change.
| Args: | ||
| source_video_path: Path to the source video file | ||
| target_video_path: Path to the target video file (output) | ||
| roboflow_api_key: Roboflow API key |
There was a problem hiding this comment.
The documentation incorrectly describes roboflow_api_key as "Roboflow API key" without noting that it can be None and will fall back to the environment variable ROBOFLOW_API_KEY. The function logic on lines 202-209 shows that roboflow_api_key can be None and will use os.environ.get("ROBOFLOW_API_KEY", api_key) as a fallback. The documentation should clarify that this parameter is optional when the environment variable is set.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2059 +/- ##
=======================================
Coverage 52% 52%
=======================================
Files 61 61
Lines 7077 7076 -1
=======================================
Hits 3657 3657
+ Misses 3420 3419 -1 🚀 New features to boost your workflow:
|
|
run locally and all seems to work as expected 🐰 |
This pull request refactors the command-line interface (CLI) handling in both
inference_example.pyandultralytics_example.pyfor the traffic analysis examples. The main improvement is replacing the previousargparse-based CLI with a more flexible approach that prefersjsonargparseif available, while providing a fallback to positional arguments for environments wherejsonargparseis not installed. Additionally, the main execution logic is moved into a dedicatedmainfunction in each script, improving modularity and readability.CLI refactoring and flexibility:
argparse-based CLI with logic that first attempts to usejsonargparsefor argument parsing, falling back to simple positional arguments ifjsonargparseis unavailable. This makes the scripts more robust and user-friendly in different environments. [1] [2]mainfunction in bothinference_example.pyandultralytics_example.py, improving code organization and enabling easier reuse and testing. [1] [2]Imports and dependencies:
argparseand addsys, reflecting the new CLI handling approach. [1] [2]Part of #2090