-
Notifications
You must be signed in to change notification settings - Fork 16
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Updated various CDF attributes/variable names to match expected nomenclature #813
Updated various CDF attributes/variable names to match expected nomenclature #813
Conversation
UNITS: ' ' | ||
VALIDMIN: 0 | ||
VALIDMAX: 127 | ||
VALIDMAX: 128 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you sure you want this to be 128? Just double checking
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I pulled this from the L1A validation CDFs that joey gave me but I do think it should be 127. I will double check with Joey.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Joey confirmed that this should be 127
(and that the exact values in his validation CDFs should not be considered the strict requirement).
"energy": energy_steps, | ||
"energy_label": energy_label, | ||
"esa_step": esa_step, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If you remove those labels, you are gonna get error for data whose dimension have more than epoch
as dimension when we need to update cdflib
. To test it, some of us have been updating cdflib
locally and then run the same test and it will raise those helpful errors. Can you do the same?
Eg.
dims = ["epoch", "inst_az", "spin_sector", "esa_step"]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah yes, I forgot that I added energy_label
in anticipation of the cdflib
upgrade in the first place. But I think I found a new (better) solution, which is to define the LABL_PTR_[1|2|3]
s in the attribute YAML file. It appears to work in both cdflib
1.3
and 1.2
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good to know. I am surprised that you were able to reuse dimension cdf attrs for labels like this
LABL_PTR_1: inst_az
LABL_PTR_2: spin_sector
LABL_PTR_3: esa_step
I thought it has to be metadata type.
Change Summary
Overview
This PR updates some of the CoDICE CDF attribute and variable names to match the expected nomenclature that is documented here: https://lasp.colorado.edu/galaxy/display/IMAP/IMAP+Nomenclature+of+Science+Data+Products. I also changed a few values in order to be more consistent with the CoDICE L1a validation CDFs I received.
Updated Files
imap_processing/cdf/config/imap_codice_l1a_variable_attrs.yaml
imap_processing/codice/codice_l1a.py
Closes #774