Skip to content

Conversation

@maxinelasp
Copy link
Contributor

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

  • Add CLI.py management of the l1b conversion table ancillary file
  • Add a xr.Dataset->AncillaryParameters conversion step
  • Pass around AncillaryParameters where needed.

Testing

  • Explicitly defined the values for the test from the variables

Copy link
Contributor

Copilot AI left a 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.

Copy link
Collaborator

@greglucas greglucas left a 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.

@maxinelasp
Copy link
Contributor Author

@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.

@maxinelasp maxinelasp merged commit abad717 into IMAP-Science-Operations-Center:dev Jan 29, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants