GLOWS L1A Attributes: Merge updates from Marek#2848
GLOWS L1A Attributes: Merge updates from Marek#2848maxinelasp merged 4 commits intoIMAP-Science-Operations-Center:devfrom
Conversation
There was a problem hiding this comment.
Pull request overview
Updates the GLOWS L1A CDF variable-attribute configuration to reflect revised metadata values (descriptions, fill values, formats, plot/display hints) based on an updated attributes file discussed with Marek.
Changes:
- Adjusts multiple variable metadata fields (e.g., CATDESC/FIELDNAM/LABLAXIS/FORMAT/UNITS) for GLOWS L1A products.
- Updates several fill values and valid ranges (notably for flags/support-data and spin/bin related fields).
- Changes display intent for the histogram variable (time_series → spectrogram).
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
|
pre-commit.ci autofix |
|
pre-commit.ci run |
maxinelasp
changed the title
tech3371
left a comment
tech3371
left a comment
There was a problem hiding this comment.
I reviewed this Since Sean and David are both helping with Lo this month.
| LABLAXIS: Counts | ||
| FILLVAL: *max_uint16 | ||
| FORMAT: I4 | ||
| LABLAXIS: Bin no. |
There was a problem hiding this comment.
. at the end may be intentional by you but can you spell it out instead? Eg. Bin Number? Same comment for all other label axis fields.
There was a problem hiding this comment.
LABLAXIS has to be < 7 characters, I believe. That was why it is not spelled out.
| LABLAXIS: Counts | ||
| VALIDMAX: *max_uint32 | ||
| LABLAXIS: Direct events | ||
| VALIDMAX: *max_uint32_min_one |
There was a problem hiding this comment.
Thank you for making alphabetic order!
There was a problem hiding this comment.
That was all Sean!
| CATDESC: Histogram start time, IMAP-clock seconds | ||
| FIELDNAM: Histogram start time, IMAP-clock seconds | ||
| CATDESC: Histogram start time (IMAP clock) | ||
| FIELDNAM: Start time (IMAP clock) |
There was a problem hiding this comment.
Does all these (IMAP clock) mean Spacecraft clock? If so, should we call it that? Eg. (IMAP Spacecraft Clock)
There was a problem hiding this comment.
Yes, it's the spacecraft clock. I think IMAP clock is clear - are there other instruments that use IMAP Spacecraft Clock?
| LABLAXIS: Duration | ||
| UNITS: seconds | ||
| VALIDMAX: 4000.0 # 15.38 s per spin x 256 spins = 3937.3 s, then rounded up | ||
| CATDESC: Accumulation time for histogram (GLOWS clock) |
There was a problem hiding this comment.
GLOWS instrument has an internal clock
| FORMAT: F10.6 | ||
| LABLAXIS: Accum. time | ||
| UNITS: s | ||
| VALIDMAX: 999.0 # 15.38 s per spin x 64 spins = 984.32 s, then rounded up |
| CATDESC: Latest PPS arrival time (GLOWS clock, seconds) | ||
| DISPLAY_TYPE: no_plot | ||
| FIELDNAM: GLOWS-clock seconds for last PPS | ||
| FIELDNAM: Latest PPS time (GLOWS seconds) |
There was a problem hiding this comment.
These (GLOWS clock, seconds) or (GLOWS seconds) can cause confusion later. Can you keep same since you are below character limits and have room to spell out.
There was a problem hiding this comment.
The GLOWS team requested these changes, so I think they prefer it with the parentheses.
tech3371
left a comment
tech3371
left a comment
There was a problem hiding this comment.
sounds good since it seems like it's thought through.
LGTM. You can merge once pre-commit is fixed!
maxinelasp
merged commit ac62cdd
into
IMAP-Science-Operations-Center:dev
Change Summary
This is an AI-generated change which updates the L1A attributes according to the file sent by Marek. We discussed these changes in a meeting, so they were reviewed by both of us already. This change does NOT touch any of the attribute names, so it should only affect the metadata.