-
Notifications
You must be signed in to change notification settings - Fork 22
Updates to include missing ancillary file for glows #2630
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
Updates to include missing ancillary file for glows #2630
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 pull request adds support for the missing l1b conversion table ancillary file for GLOWS processing. The conversion table is now loaded from a JSON file via the CLI and passed through the processing pipeline instead of being hardcoded within the processing functions.
Changes:
- Added CLI management for loading the l1b conversion table ancillary file and passing it to both histogram and direct event processing
- Updated function signatures throughout the codebase to pass conversion_table_dict/ancillary_parameters where needed
- Removed hardcoded file loading from DirectEventL1B.post_init and replaced it with a parameter-based approach
- Updated test fixtures and test function signatures to include mock_conversion_table_dict
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| imap_processing/cli.py | Loads conversion table JSON file and passes it to glows_l1b() and glows_l1b_de() functions |
| imap_processing/glows/l1b/glows_l1b.py | Updated function signatures to accept conversion_table_dict; created closure for passing ancillary_parameters in process_de() |
| imap_processing/glows/l1b/glows_l1b_data.py | Added ancillary_parameters as InitVar parameter to DirectEventL1B; removed hardcoded file loading; added physical_unit to validation |
| imap_processing/tests/glows/conftest.py | Added mock_conversion_table_dict fixture with physical_unit keys; updated fixtures to pass conversion table through |
| imap_processing/tests/glows/test_glows_l1b.py | Updated test function signatures to include mock_conversion_table_dict parameter |
| imap_processing/tests/glows/test_glows_l1b_data.py | Updated test function signatures to include mock_conversion_table_dict parameter |
| imap_processing/tests/glows/test_glows_l2.py | Updated test function signatures to include mock_conversion_table_dict parameter |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
greglucas
left a comment
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.
Looks good. There are a lot of places in code you are adding this pass-through so I'm not sure if there would be a better way to bundle this up beforehand to avoid needing to add it in so many locations, say if you get yet another ancillary file or something along those lines.
|
@greglucas I agree, but I don't expect to get more files (this is not a change from GLOWS, rather just something I missed when I set up the rest of the inputs for whatever reason) so hopefully this is the last edit. |
abad717
into
IMAP-Science-Operations-Center:dev
Change Summary
During the ancillary file updates for GLOWS, I missed one. This PR will have a matching one to add the dependnecy in sds-data-manager. It primarily just wires up the file to the already-existing dataclass for passing the information around, so not a major change.
Overview
Testing