Conversation
There was a problem hiding this comment.
🟡 Changes recommended
The shared attribute removal incorrectly detaches CoDICE Lo L1B energy variables and labels from their esa_step dimension.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Fixes CoDICE L2 solar-wind species energy-axis metadata.
Changes:
- Swaps the species dimension to physical energy-per-charge.
- Adds CDF round-trip and dependency assertions.
- Updates CDF attributes and generated version metadata.
File summaries
| File | Description |
|---|---|
codice_l2.py |
Switches the L2 species energy dimension. |
test_codice_l2.py |
Tests CDF loading and dependencies. |
imap_codice_l1b_variable_attrs.yaml |
Removes L1B energy dependencies. |
_version.py |
Updates generated version metadata. |
Review details
Suppressed comments (1)
imap_processing/cdf/config/imap_codice_l1b_variable_attrs.yaml:19
- The label is also produced on
esa_stepfor every CoDICE Lo L1B product (codice_l1b.py:77-89), so deleting this dependency leaves those L1B labels detached from their axis. Restore the L1B metadata; the L2 branch explicitly changes the copied label's dependency toenergy_per_charge.
CATDESC: Energy per charge
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
leowerneck
left a comment
There was a problem hiding this comment.
LGTM. I approved because I'm not sure the imap_processing/_version.py change is supposed to be made or not (my understanding is it shouldn't be updated). In either case, the fix (if required) it simple enough that doesn't warrant a new review.
bfbd0d3
into
IMAP-Science-Operations-Center:dev
Change Summary
closes #3464
Overview
Fix the attribute dimension switch.