Fix model data writer and avoid overwrite of general output path #2000
Conversation
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
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where ModelDataWriter was changing the global output path for all modules by modifying the IOHandler singleton's output_path. The fix introduces a label-based dictionary approach in IOHandler to manage multiple output paths independently, and simplifies the ModelDataWriter API.
Changes:
- Modified IOHandler to use a dictionary for output_path with label-based access instead of a single path
- Simplified ModelDataWriter API by removing args_dict parameter and renaming product_data_file/product_data_format to output_file/output_file_format
- Updated all callers to pass individual parameters (output_file, output_file_format) instead of args_dict
Reviewed changes
Copilot reviewed 13 out of 13 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/simtools/io/io_handler.py | Changed output_path from single value to dictionary with label-based access; added output_path_label parameter to set_paths, get_output_directory, and get_output_file methods |
| src/simtools/data_model/model_data_writer.py | Simplified init signature; renamed product_data_* to output_file_*; removed args_dict parameter from dump(); added settings.config usage; enhanced _astropy_data_format to derive format from file suffix |
| tests/unit_tests/io/test_io_handler.py | Updated test to access output_path as dictionary |
| tests/unit_tests/data_model/test_model_data_writer.py | Updated tests for renamed attributes; added settings.config.load() calls; added tests for _astropy_data_format with None; removed one test case for explicit output_file |
| tests/unit_tests/configuration/test_configurator.py | Updated test to access output_path as dictionary |
| src/simtools/ray_tracing/mirror_panel_psf.py | Updated dump() call to pass individual parameters instead of args_dict |
| src/simtools/ray_tracing/incident_angles.py | Simplified ModelDataWriter instantiation to only pass output_file |
| src/simtools/camera/single_photon_electron_spectrum.py | Updated dump() call to pass individual parameters |
| src/simtools/applications/submit_data_from_external.py | Updated dump() call to pass individual parameters |
| src/simtools/applications/generate_regular_arrays.py | Updated ModelDataWriter instantiation with renamed parameters |
| src/simtools/applications/db_get_array_layouts_from_db.py | Updated dump() call to pass individual parameters |
| src/simtools/applications/convert_geo_coordinates_of_array_elements.py | Updated dump() call to pass individual parameters with default for format |
| docs/changes/2000.maintenance.md | Added changelog entry |
|
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.
Closes #1997
ModelDataWriter changes the general output path (the one retrieved with
IoHandler::get_output_directory). This should not happen.Fix this with this PR:
Remove also temporary statement mentioned here.