Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a command-line interface for NetGraph, allowing users to run scenario files and serialize results to JSON.
- Add
ngraph.climodule with arunsubcommand for executing scenarios and outputting JSON. - Expose CLI entry points in
__main__.py,__init__.py, and register thengraphconsole script inpyproject.toml. - Extend
Resultswith ato_dictmethod and add tests covering file- and stdout-based CLI output.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_cli.py | Add tests for run subcommand writing to file and stdout |
| pyproject.toml | Register ngraph console script under [project.scripts] |
| ngraph/results.py | Implement to_dict for serializing all stored results |
| ngraph/cli.py | Define main and _run_scenario for CLI behavior and JSON output |
| ngraph/main.py | Enable python -m ngraph entry point |
| ngraph/init.py | Expose cli in package exports |
|
|
||
| def to_dict(self) -> Dict[str, Dict[str, Any]]: | ||
| """Return a dictionary representation of all stored results.""" | ||
| return {step: data.copy() for step, data in self._store.items()} |
There was a problem hiding this comment.
The to_dict method performs a shallow copy of each inner result dict. If nested mutable values are present, consider using copy.deepcopy or documenting that nested structures remain shared.
| return {step: data.copy() for step, data in self._store.items()} | |
| return {step: copy.deepcopy(data) for step, data in self._store.items()} |
|
|
||
|
|
||
| def test_cli_run_file(tmp_path: Path) -> None: | ||
| scenario = Path("tests/scenarios/scenario_1.yaml") |
There was a problem hiding this comment.
Hardcoding the scenario path assumes the current working directory; consider deriving it with Path(__file__).parent / "scenarios" / "scenario_1.yaml" for more robust test location resolution.
| scenario = Path("tests/scenarios/scenario_1.yaml") | |
| scenario = Path(__file__).parent / "scenarios" / "scenario_1.yaml" |
| results_dict: Dict[str, Dict[str, Any]] = scenario.results.to_dict() | ||
| json_str = json.dumps(results_dict, indent=2, default=str) | ||
| if output: | ||
| output.write_text(json_str) |
There was a problem hiding this comment.
[nitpick] Writing JSON without a trailing newline can break POSIX conventions; consider appending "\n" to json_str when calling write_text.
| output.write_text(json_str) | |
| output.write_text(json_str + "\n") |
1 participant
Summary
ngraph.climodule with a basic command-line interface__main__and package__init__Resultsto JSONpyproject.tomlTesting
ruff check ngraph/cli.py ngraph/__init__.py ngraph/__main__.py ngraph/results.py tests/test_cli.pyblack --check ngraph/cli.py ngraph/__init__.py ngraph/__main__.py ngraph/results.py tests/test_cli.pyisort --check-only ngraph/cli.py ngraph/__init__.py ngraph/__main__.py ngraph/results.py tests/test_cli.pypytest -q