HIT L2 - ISTP compliance updates#1852
Merged
vmartinez-cu merged 13 commits intoIMAP-Science-Operations-Center:devfrom Jun 30, 2025
Merged
HIT L2 - ISTP compliance updates#1852vmartinez-cu merged 13 commits intoIMAP-Science-Operations-Center:devfrom
vmartinez-cu merged 13 commits intoIMAP-Science-Operations-Center:devfrom
Conversation
…ency with logical source value
…ve DEPEND_1 in energy mean variables. Remove DISPLAY_TYPE for support data variables
-Add SPASE metadata to support data variables -Update energy variable units to energy per charge -Remove display type for uncertainty variables -Update energy variable CATDESC -Update dynamic threshold state fillval
…ta type. Replace DEPEND_0 with DEPEND_1 for a few energy variables to match other energy variable attributes. Fix typo. Update format for energy variables
…ing DEPEND_1 as an attribute to the label which isn't needed for metadata variables. Checking with Andriy for clarification on dimensions for labels
vmartinez-cu
added
Ins: HIT
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR updates several modules for improved compliance with ISTP standards by addressing typos and enhancing attribute configurations. Key changes include:
- Adjusting dimension naming for label arrays and converting state variables to np.int8.
- Modifying YAML configuration entries for energy and uncertainty attributes, including renaming keys and updating field descriptions.
- Fixing a typo in the global CDF attributes file.
Reviewed Changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| imap_processing/hit/l2/hit_l2.py | Updates dimension naming for label arrays and applies np.int8 conversion for dynamic threshold state. |
| imap_processing/hit/l0/decom_hit.py | Sets the appropriate dtype based on field type for parsed data arrays. |
| imap_processing/hit/hit_utils.py | Rounds energy values to one decimal place and converts them to np.float32. |
| imap_processing/cdf/config/imap_hit_l2_variable_attrs.yaml | Adjusts numerous variable attributes (e.g., DELTA_MINUS_VAR/PLUS_VAR, FIELDNAM capitalization) for consistency with ISTP guidelines. |
| imap_processing/cdf/config/imap_hit_global_cdf_attrs.yaml | Corrects a typo in the Data_type for macropixel intensity. |
Comments suppressed due to low confidence (4)
imap_processing/hit/l2/hit_l2.py:140
- Ensure that changing the dimension name to a formatted string (i.e. appending '_label') aligns with the expectations of cdflib and any downstream consumers.
dims=[f"{dim}_label"],
imap_processing/hit/l2/hit_l2.py:318
- Verify that converting 'dynamic_threshold_state' to np.int8 correctly represents the expected range for the state values and does not truncate critical information.
].astype(np.int8)
imap_processing/cdf/config/imap_hit_l2_variable_attrs.yaml:69
- Confirm that the change from using 'DELTA_MINUS' to 'DELTA_MINUS_VAR' (and similarly for DELTA_PLUS) is properly reflected in all consuming modules and documentation.
DELTA_MINUS_VAR: h_energy_delta_minus
imap_processing/cdf/config/imap_hit_l2_variable_attrs.yaml:505
- [nitpick] Review the naming consistency for FIELDNAM values (e.g. the change to lowercase 'intensity summed'); ensure that this change is intentional and compatible with any consumers that expect a specific case.
FIELDNAM: H intensity summed
greglucas
approved these changes
Jun 30, 2025
| else: | ||
| dims = ["epoch"] | ||
|
|
||
| dtype = np.int8 if field == "hdr_dynamic_threshold_state" else np.int64 |
Collaborator
There was a problem hiding this comment.
"state" variables are usually unsigned ints, should this be "uint8" instead?
Collaborator
Author
There was a problem hiding this comment.
Yes, good catch!
… for nucleon with full spelling in CATDESC attributes where needed for clarity
vmartinez-cu
merged commit b4b02d0
into
IMAP-Science-Operations-Center:dev
13 of 14 checks passed
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.
This PR incorporates feedback from Andriy with SPDF to improved attributes for compliance with ISTP standards.
Updated Files