-
Notifications
You must be signed in to change notification settings - Fork 22
Update common map variable metadata #2279
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
Update common map variable metadata #2279
Conversation
|
Looks good to me. Ultra has some assertions on exact metadata it looks like, could you update those tests here too. |
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.
Thank you for propagating many changes from the spreadsheet into this YAML. I have a handful of changes that are needed.
| # in the tiling-specific YAML files. | ||
| longitude: | ||
| <<: *default_float32 | ||
| CATDESC: "Pixel center longitude in {frame} reference frame in the range [0, 360]." |
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 specifically put this in so that the reference frame could be injected into this attribute. See: #2242
| LABLAXIS: "{frame} Longitude" | ||
| UNITS: degrees | ||
| LABLAXIS: Longitude | ||
| FORMAT: I3 |
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.
This is the common variable attributes file meaning that these attributes get included in the Ultra HealPix maps as well. Since HealPix maps don't have integer lat/lon, I think this belongs in the YAML file specifically for RectangularMaps.
| <<: *default_float32 | ||
| CATDESC: "Pixel center longitude in {frame} reference frame in the range [0, 360]." | ||
| CATDESC: Pixel center longitude | ||
| FIELDNAM: "{frame} Longitude" |
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.
SPDF gave specific guidance on this:
For longitude, and latitude variables, please include coordinate system name (HAE ?) in all of their CATDESC, FIELDNAM and LABLAXIS, so that, for example, LABLAXIS="HAE latitude", so that immediately visible to user.
That is why I put these {frame} placeholders in here. They will eventually be populated.
| LABL_PTR_1: longitude_label | ||
| SCALETYP: linear | ||
| MONOTON: INCREASE | ||
| DELTA_PLUS_VAR: longitude_delta | ||
| DELTA_MINUS_VAR: longitude_delta |
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.
Again, these need to be moved to the Rectangular specific YAML file: imap_enamaps_l2-rectangular_variable_attrs.yaml
| DEPEND_0: epoch | ||
| VAR_TYPE: support_data | ||
| LABLAXIS: Non-Statistical Err | ||
| LABLAXIS: Non-statistical Error |
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.
SPDF guidance for LABLAXIS:
approximately 10 characters, but preferably 6 characters
| datatype: int64 | ||
| CATDESC: Standard deviation of the observation date. | ||
| FIELDNAM: Observation date range | ||
| FIELDNAM: J2000 Nanoseconds |
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.
Even though this is what is entered in the spreadsheet, I don't think this is correct for FIELDNAM
…, made other map variable metadata tweaks
|
Hi Tim, thanks for looking at this. We made some updates. Let us know if anything else needs to change. We also updated the spreadsheet, updates are highlighted in red. |
subagonsouth
left a comment
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.
Thanks! LGTM.
6fd38b5
into
IMAP-Science-Operations-Center:dev
Change Summary
Updated the common map metadata per the definition here
Note that many of the variables in the green section of the spreadsheet were missing from the existing definition and these were not added in this PR. (bg_rate_uncert, bg_intensity, ...). We believe ena_rate, and ena_count have been renamed to ena_count_rate, and counts, but believe this change will require updates to the code so is not included here.
Updated Files