gbrammer / grizli

Grizli: The "Grism redshift and line" analysis software
MIT License
67 stars 51 forks source link

Bug with `DATE` keyword in the WFSS background files #198

Closed gbrammer closed 9 months ago

gbrammer commented 9 months ago

@TheSkyentist The recent updates to the WFSS background file parsing and copying now break when the WFSS background files don't have the DATE keyword specified. I don't see where that keyword is set when making the files for the first time.

Can you update the logic at the location below to handle cases where the keyword is not found and also insert somewhere where it is set the first time the file is created?

https://github.com/gbrammer/grizli/blob/73bb3eef89fc97c1241c76e13e6fc6c23b20b545/grizli/prep.py#L6246

gbrammer commented 9 months ago

Perhaps the DATE keyword should be added below here, though I'm not sure quite what you intend with the keyword and with the test that the file is "up to date":

https://github.com/gbrammer/grizli/blob/73bb3eef89fc97c1241c76e13e6fc6c23b20b545/grizli/prep.py#L6275-L6280

TheSkyentist commented 9 months ago

I can see why this might create an issue. I was attempting to future proof based on the following assumptions:

1) The Flat-Divided WFSSBackground file was created using the CRDS WFSSBackground file as a starting point as is done in grizli.

2) If a new CRDS WFSS bkg file is made available, then the flat-divided version should be re-made. All CRDS WFSS BKG files ship with a 'DATE' header keyword, so if a new one is made available, then the flat-divided we store locally will not match.

However, if the user provides a custom WFSS BKG file that is not derived from a CRDS WFSS BKG file, then the DATE header keyword will always be incorrect (or non-existent).

I imagine that we should probably remove the hard DATE requirement, and simply pass a warning if the DATE keyword doesn't exist or is different than the CRDS WFSS file, at least letting the user know what is going on.

Thoughts?

gbrammer commented 9 months ago

Yes, I think that's a good idea to also consider no DATE keyword found as a valid test that the file is acceptable and have other logic to test the DATE if it is found.

gbrammer commented 9 months ago

https://github.com/gbrammer/grizli/pull/199