Fix CoDICE direct-events DISPLAY_TYPE and dependency metadata#3202
Open
davidt0x wants to merge 2 commits into
Open
Fix CoDICE direct-events DISPLAY_TYPE and dependency metadata#3202davidt0x wants to merge 2 commits into
davidt0x wants to merge 2 commits into
Conversation
…labels Fix the remaining CoDICE direct-events metadata issues in imap_codice_l2_lo-direct-events and imap_codice_l2_hi-direct-events that were causing SPDF DISPLAY_TYPE failures and stale label-metadata conflicts in the written L2 CDFs. The main problem was that many direct-event variables were missing an explicit DISPLAY_TYPE in the L2 attr templates, and the direct-event L2 builders were merging L2 attrs onto inherited L1A attrs with `attrs.update(...)`. Because `update()` does not remove keys, stale L1A metadata such as LABLAXIS survived into the L2 output even when the L2 template intentionally omitted it. That created two classes of issues: - SPDF failures for missing or invalid DISPLAY_TYPE metadata - write-time failures with newer cdflib when a variable ended up with both LABLAXIS and LABL_PTR_* in the final L2 attrs Update the direct-event L2 attr templates to add explicit DISPLAY_TYPE values using a mixed policy: - time_series for num_events - spectrogram for plottable multidimensional physical variables such as tof, spin_angle, elevation_angle, energy_per_charge, apd_energy, ssd_energy, and energy_per_nuc - no_plot for categorical/index/support variables such as data_quality, gain, multi_flag, type, apd_id, position, energy_step, and ssd_id Also remove LABLAXIS from the direct-event data variables that already use LABL_PTR_* label metadata. This matches the CoDICE/CAVA guidance captured in issue IMAP-Science-Operations-Center#2765 and avoids the LABLAXIS/LABL_PTR conflict during CDF writing. In the direct-event L2 builders, replace inherited variable attrs with the L2 template attrs instead of merging them. This keeps the L2 attr template as the source of truth for normal direct-event data variables and prevents stale L1A-only keys from surviving in memory. The existing skip logic for nso*/rgfo* variables is preserved. Add a parameterized written-CDF regression test that: - generates fresh lo-direct-events and hi-direct-events L2 files - inspects the written CDF with cdflib - verifies the affected variables have DISPLAY_TYPE present - verifies DISPLAY_TYPE is stored as a string - verifies the emitted DISPLAY_TYPE values match the intended policy This also restores the normal L2 write_cdf() path in the direct-events tests after removing the temporary workaround that had disabled strict ISTP writing for these products. Validated locally with: - focused CoDICE direct-events pytest runs - pre-commit on the touched files - SPDF checks on regenerated lo-direct-events and hi-direct-events files After this change, the regenerated direct-events files no longer show the old DISPLAY_TYPE / ClassCastException failures, and the LABLAXIS / LABL_PTR conflict is cleared. Other unrelated CoDICE ISTP issues remain out of scope for this commit.
Fix the remaining CoDICE direct-events metadata mismatch in `imap_codice_l2_hi-direct-events`. `energy_per_nuc` is a multidimensional data variable whose second axis is the numeric `priority` coordinate. Its L2 attr template was incorrectly setting: DEPEND_1 = priority_label That made the dependency point at a character label variable rather than the actual coordinate variable, which caused SPDF to report: DEPEND_1 is a character type Update the L2 attr template so `energy_per_nuc` follows the same ISTP pattern already used by neighboring direct-events variables: - `DEPEND_1 = priority` - keep `LABL_PTR_1 = priority_label` This preserves the label mapping for display purposes while making the dimension dependency point to the correct numeric support variable. Also extend the existing direct-events written-CDF metadata regression to verify that `hi-direct-events` writes: - `energy_per_nuc DEPEND_1 = priority` - `energy_per_nuc LABL_PTR_1 = priority_label` Validated locally with: - focused CoDICE direct-events pytest runs - pre-commit on the touched files This commit intentionally leaves the separate `epoch_delta_*` metadata question untouched.
tech3371
approved these changes
May 15, 2026
Contributor
tech3371
left a comment
tech3371
left a comment
There was a problem hiding this comment.
Thank you for finding those and fixing them!
| ] | ||
| processed_l2_ds = process_codice_l2(descriptor, ProcessingInputCollection()) | ||
| processed_l2_ds.attrs["Data_version"] = "001" | ||
| return write_cdf(processed_l2_ds) |
Contributor
There was a problem hiding this comment.
That's how I test with CDF attrs too
|
|
||
| @pytest.mark.parametrize("descriptor", ["lo-direct-events", "hi-direct-events"]) | ||
| @patch("imap_data_access.processing_input.ProcessingInputCollection.get_file_paths") | ||
| def test_codice_l2_direct_events_display_type_cdf_metadata( |
Contributor
There was a problem hiding this comment.
nice general test for both direct event
Contributor
There was a problem hiding this comment.
Pull request overview
This PR finalizes CoDICE L2 direct-events metadata cleanup to satisfy SPDF/CAVA/ISTP expectations by making DISPLAY_TYPE explicit, removing stale LABLAXIS conflicts, and correcting a bad dependency pointer for energy_per_nuc.
Changes:
- Updated CoDICE direct-events L2 variable-attribute YAML templates to add explicit
DISPLAY_TYPEvalues and remove problematicLABLAXISentries where label pointers are used. - Adjusted direct-events L2 attribute application to replace variable attribute dictionaries (instead of merging), preventing stale/inherited conflicts from persisting into written CDFs.
- Extended regression tests to validate written-CDF metadata (
DISPLAY_TYPE, andenergy_per_nucDEPEND_1/LABL_PTR_1).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
imap_processing/tests/codice/test_codice_l2.py |
Adds a written-CDF metadata regression test for direct-events DISPLAY_TYPE and energy_per_nuc dependency metadata. |
imap_processing/codice/codice_l2.py |
Changes direct-events L2 variable attribute application to overwrite attrs from templates (to avoid inherited metadata conflicts). |
imap_processing/cdf/config/imap_codice_l2-lo-direct-events_variable_attrs.yaml |
Adds explicit DISPLAY_TYPE values and removes stale LABLAXIS entries that can conflict with LABL_PTR_*. |
imap_processing/cdf/config/imap_codice_l2-hi-direct-events_variable_attrs.yaml |
Adds explicit DISPLAY_TYPE values, removes stale LABLAXIS, and corrects energy_per_nuc DEPEND_1 to point at priority. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+800
to
+814
|
|
||
| for variable, expected_display_type in DIRECT_EVENT_DISPLAY_TYPES[ | ||
| descriptor | ||
| ].items(): | ||
| attrs = cdf_file.varattsget(variable) | ||
| assert "DISPLAY_TYPE" in attrs, f"{variable} is missing DISPLAY_TYPE" | ||
| assert isinstance(attrs["DISPLAY_TYPE"], str), ( | ||
| f"{variable} DISPLAY_TYPE must be stored as a string" | ||
| ) | ||
| assert attrs["DISPLAY_TYPE"] == expected_display_type | ||
|
|
||
| if descriptor == "hi-direct-events": | ||
| attrs = cdf_file.varattsget("energy_per_nuc") | ||
| assert attrs["DEPEND_1"] == "priority" | ||
| assert attrs["LABL_PTR_1"] == "priority_label" |
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
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.
Summary
This PR completes the current CoDICE direct-events metadata cleanup for:
imap_codice_l2_lo-direct-eventsimap_codice_l2_hi-direct-eventsParent issue: #2577
Related CoDICE metadata issue: #2765
What Changed
Add explicit
DISPLAY_TYPEmetadataThe direct-events L2 attr templates were missing explicit
DISPLAY_TYPEvalues on a set of multidimensional data/support variables. This caused SPDF/CAVA validation failures and inconsistent metadata in the written CDFs.This PR adds explicit string
DISPLAY_TYPEvalues using the direct-events policy already validated on this branch:time_seriesfornum_eventsspectrogramfor plottable multidimensional physical variablesno_plotfor categorical/index/support variablesRemove stale
LABLAXISconflictsThe direct-events products were also carrying stale label metadata that could leave variables with both
LABLAXISandLABL_PTR_*, which newercdflibrejects during CDF writing.This PR cleans up the attr templates and direct-events L2 attr application so the written L2 products no longer retain those stale inherited conflicts.
Fix
energy_per_nucdependency metadataenergy_per_nucinimap_codice_l2_hi-direct-eventshad:DEPEND_1 = priority_labelThat incorrectly pointed the dependency at a character label variable rather than the numeric coordinate variable, which caused SPDF to report:
DEPEND_1 is a character typeThis PR corrects that to:
DEPEND_1 = prioritywhile keeping:
LABL_PTR_1 = priority_labelSo the display label is preserved, but the dependency points to the correct numeric support variable.
Testing
Extended the written-CDF direct-events regression coverage in
test_codice_l2.pyto verify:DISPLAY_TYPEhi-direct-eventswrites:energy_per_nuc DEPEND_1 = priorityenergy_per_nuc LABL_PTR_1 = priority_labelValidation performed locally:
python -m pytest -q imap_processing/tests/codice/test_codice_l2.py -k 'lo_de or hi_de or direct_events' python -m pre_commit run --files \ imap_processing/cdf/config/imap_codice_l2-hi-direct-events_variable_attrs.yaml \ imap_processing/cdf/config/imap_codice_l2-lo-direct-events_variable_attrs.yaml \ imap_processing/codice/codice_l2.py \ imap_processing/tests/codice/test_codice_l2.py