Skip to content

HIT L2 - Fix delta var attrs#3086

Merged
vmartinez-cu merged 1 commit intoIMAP-Science-Operations-Center:devfrom
vmartinez-cu:hit-fix-delta-attrs
Apr 29, 2026
Merged

HIT L2 - Fix delta var attrs#3086
vmartinez-cu merged 1 commit intoIMAP-Science-Operations-Center:devfrom
vmartinez-cu:hit-fix-delta-attrs

Conversation

@vmartinez-cu
Copy link
Copy Markdown
Collaborator

Fixes error in HIT L2 metadata where delta var attributes had reversed values assigned (minus and plus values assigned to the opposite delta attrs)

Closes Issue #2729

Copy link
Copy Markdown
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

Fixes HIT L2 CDF metadata in the variable-attributes YAML where DELTA_MINUS_VAR and DELTA_PLUS_VAR were previously assigned to the wrong uncertainty variables, aligning the DELTA links with the intended “minus” vs “plus” total-uncertainty fields (Issue #2729).

Changes:

  • Swap DELTA_MINUS_VAR to reference *_total_uncert_minus (instead of *_total_uncert_plus) for HIT L2 intensity variables.
  • Swap DELTA_PLUS_VAR to reference *_total_uncert_plus (instead of *_total_uncert_minus) for HIT L2 intensity variables.
  • Apply the correction consistently across standard, summed, and macropixel intensity variable blocks.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +352 to +353
DELTA_MINUS_VAR: h_total_uncert_minus
DELTA_PLUS_VAR: h_total_uncert_plus
Copy link

Copilot AI Apr 27, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This YAML change fixes a metadata mapping bug, but there doesn’t appear to be a regression test asserting that HIT L2 intensity variables’ DELTA_MINUS_VAR/DELTA_PLUS_VAR attrs point to the expected *_total_uncert_minus/*_total_uncert_plus variables. Consider adding a small unit test (e.g., in imap_processing/tests/hit/test_hit_l2.py) that runs hit_l2 (or add_cdf_attributes) and asserts the corrected DELTA attrs for at least one representative variable (standard, summed, macropixel) to prevent this from silently regressing again.

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unit tests don't currently validate CDF attribute values. This could be considered in the future

@vmartinez-cu vmartinez-cu added Ins: HIT Related to the HIT instrument Level: L2 Level 2 processing CDF Related to CDF files labels Apr 27, 2026
Copy link
Copy Markdown
Contributor

@sdhoyt sdhoyt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice!

@vmartinez-cu vmartinez-cu merged commit b5fd291 into IMAP-Science-Operations-Center:dev Apr 29, 2026
26 of 27 checks passed
@vmartinez-cu vmartinez-cu deleted the hit-fix-delta-attrs branch April 29, 2026 05:03
maxinelasp pushed a commit to maxinelasp/imap_processing that referenced this pull request May 1, 2026
lacoak21 pushed a commit to lacoak21/imap_processing that referenced this pull request May 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CDF Related to CDF files Ins: HIT Related to the HIT instrument Level: L2 Level 2 processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants