standardize getting NIGHT from raw data headers #1083
Conversation
| #- update header as needed so the correction propagates downstream | ||
| try: | ||
| night = int(primary_header['NIGHT']) | ||
| except (KeyError, ValueError, TypeError): |
There was a problem hiding this comment.
If this fails, I don't think the variable "night" gets defined using this try statement.
Also, is there a motivation for using try rather than just checking that the key exists and value isn't None? I appreciate that the net result is equivalent, so I'm fine with this.
There was a problem hiding this comment.
If you don't use the variable night, I think your function call already has this logic, so it might be redundant as well. You could just call it for everything and it will return without assigning anything of the header keyword is valid.
There was a problem hiding this comment.
The point here is to test if the keyword "NIGHT" exists and whether it can be converted into an int, and if not replace it with a valid NIGHT. Although "and if not None" logic would work for the problem last night (20210105), on 20190626 the keyword existed, wasn't None, but was a blank string instead. Rather than try to individually test for each way it could be wrong in the past and future, I opted for try/except for whether you can succeed to hopefully be robust to new and novel ways in which that keyword could be wrong in the future...
You make a good point about night not being set if the try fails. I didn't actually need that variable but the meaningful name implied that maybe you could rely upon it, so I renamed it to tmp instead.
| pass | ||
|
|
||
| raise ValueError('Unable to derive YEARMMDD from header NIGHT,DATE-OBS,MJD') | ||
|
|
There was a problem hiding this comment.
Same comment here as before about try's vs if's. Fine either way
|
Thanks for updating offline pipeline code. Merging now. |
This PR provides
desispec.util.header2nightwhich standardizes how to get the YEARMMDD NIGHT from the raw data fits headers, including handling real-world problems that have come up:in these cases it falls back to using
DATE-OBS, and then falls back toMJD-OBS, including handling the definition of "NIGHT" rolling over at noon MST (KPNO local time).dateobs2nightandmjd2nightare also added as utility functions used byheader2night. These might be useful by other code too, but if you are deriving a YEARMMDD from a header, please useheader2night(header)to handle the various bogus-header cases.I have tested this with
desi_proc,desi_qproc(via nightwatch), and the unit tests, but I have not updated thedesi_daily_proc_managerbookkeeping code.@akremin please add the necessary fixes to this branch for the nightly processing bookkeeping to work too. Thanks.