-
Notifications
You must be signed in to change notification settings - Fork 3
Github Code quality #1959
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Github Code quality #1959
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR addresses code quality issues identified by GitHub Code QL, focusing on cleaning up unused code, improving import patterns, and making return statements more explicit.
Key changes:
- Refactored test imports from direct function imports to module-level imports with qualified names
- Removed unused variable declarations and logger instances
- Fixed duplicate
rst_epilogdefinition in Sphinx configuration
Reviewed changes
Copilot reviewed 19 out of 19 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit_tests/simtel/test_simulator_camera_efficiency.py | Extracted values from pop() into intermediate variables before assertions to improve code clarity |
| tests/unit_tests/simtel/test_simtel_io_metadata.py | Changed from direct function imports to module-qualified calls (e.g., simtel_io_metadata.read_sim_telarray_metadata) |
| tests/unit_tests/ray_tracing/test_psf_parameter_optimisation.py | Changed from direct import of GradientStepResult to module-qualified calls (psf_opt.GradientStepResult) |
| tests/unit_tests/production_configuration/test_derive_corsika_limits.py | Changed from direct function imports to module-qualified calls for better code clarity |
| tests/unit_tests/layout/test_telescope_position.py | Removed unused variable assignments from test assertions |
| tests/unit_tests/layout/test_array_layout_utils.py | Changed from mixed import style to consistent module-qualified calls |
| tests/unit_tests/layout/test_array_layout.py | Removed unused variable assignment in exception handling test |
| tests/unit_tests/job_execution/test_job_manager.py | Changed from direct import of JobExecutionError to module-qualified calls |
| tests/unit_tests/io/test_ascii_handler.py | Changed from direct function import to module-qualified call |
| tests/unit_tests/data_model/test_model_data_writer.py | Extracted assertion result into intermediate variable for clarity |
| tests/unit_tests/data_model/test_metadata_collector.py | Changed from direct class import to module-qualified call and removed unused assignment |
| tests/unit_tests/corsika/test_corsika_config.py | Extracted value from pop() into intermediate variable before assertion |
| src/simtools/visualization/plot_corsika_histograms.py | Removed unused logger import and instance |
| src/simtools/ray_tracing/psf_analysis.py | Removed unused variable initialization (s0, s1 = 0, 0) that was immediately overwritten |
| src/simtools/io/ascii_handler.py | Made return statements explicit by separating function calls from returns |
| src/simtools/data_model/metadata_model.py | Removed unused logger import and instance |
| src/simtools/configuration/commandline_parser.py | Removed unused return value assignment (argparse pattern doesn't require capturing returned group) |
| docs/source/conf.py | Removed duplicate rst_epilog definition that was overwriting the first one |
| docs/changes/1959.maintenance.md | Added changelog entry describing the code quality improvements |




see https://github.com/gammasim/simtools/security/quality
(not sure yet - maybe this is useful, maybe not).