Is there a sane place in the butler to put a function which modifies a header? The test code takes a fits pathname and returns a pyfits header – not sure whether such a thing can be used. price?

Try PfsMapper.std_raw and std_raw_md. See HscMapper for examples.

price when you have the opportunity, can you review the proposal please?

Looks reasonable to me.
I suggest running it by Tim Jenness (LSST), as he's thought about these kinds of things.

cloomis to add initial code to git and provide link in this ticket.

This has not changed since the tickets was opened. For evaluation, am just providing an executable, not tests, sorry. From $OBS_PFS_DIR, python/lsst/obs/pfs/patchHeader.py --yamlDir=tests/patches /your/path/to/PFLA01499... will show what would happen.

This is assigned for me, and it's not clear what I'm expected to do. I'm thinking I'm to implement this scheme in the butler?

On a second reading, I have the following comments:

> We propose saving YAML files containing a dictionaries of list of rules, in the pfs_instdata git repo.

The pfs_instdata repo is HUGE, and having to download it would compromise our continuous integration with Travis (which i think must set time limits on jobs). Can we put these in their own repo instead?

I'm not necessarily opposed to YAML files with rules, but I wonder if it might not be easier to use python code instead, with a number of standard functions defined that do the various adding/modifying/deleting of keywords?

> Are filename globs the right key?

Not sure. When we're using this in the butler, we have keyword-value pairs like visit=123 arm=r spectrograph=4. We can construct a raw filename, but that uses the PF*r1.fits-like style of filenames rather than PF*12.fits style. Of course, we can convert back to the latter style; it's just annoying.

Since this was opened Tim Jenness has implemented a viable header patching scheme for LSST, and we should probably adopt it instead.

Tim's header patching scheme is implemented in the astro_metadata_translator package, which is intended to be a general-purpose astronomy metadata translator (FITS headers to astronomical metadata), and is used by the butler to generate VisitInfo from the FITS headers. There is a fix_header function that searches for a YAML file (specified by observation ID) that sets values in the header (it has no ability to set values programmatically); this is used within the butler. The path for the YAML files can be specified explicitly, or via a envvar (METADATA_CORRECTIONS_PATH).

I think this will be sufficient for our purposes if we add a layer over the top to generate the YAML files programmatically. Tim is concerned that the YAML files be available for people to download without needing a bunch of prerequisites, so I'm hopeful that he'll move the source path to the obs_*_data packages (relatively new feature in the LSST stack, currently contains defects). Until that comes together (possibly the LSST 19.0 release?), we can build them in obs_pfs and set the envvar appropriately.

cloomis, what are the requirements from your end? Are you OK with this living in obs_pfs? Do we really need to be able to change values based on whether they are set to a particular value, or is it enough to just set it to something new (if we can define in python code what it gets set to)?

So we can replace all of the obs_pfs header->metadata functionaity by this? That sounds like a big win. Am a little surprised by how static the "fix_headers" mechanism is, and that it requires one file per input file, but it obviously could be extended if necessary. I am actually with you on preferring code for this kind of problem: a recent Subaru gotcha was that the supplied UTC timestamps were in HST for a few days; for that, being able to call out to to a function which could add 10 hours for all visits in a given range is much more sane than making a file for each.

Realistically, DRP is the only consumer. And as far as I can tell that only cares about ~20 cards (time, identity, motors, exposure times). So sure, this looks fine.

I've put an implementation in place that uses Tim Jenness' astro_metadata_translator package. Unfortunately it doesn't have any actual data in place for headers to patch, but I've checked that it does in fact patch the headers on ingest.

cloomis, please have a look, and let me know what you think. I've put your work on a new u/cloomis/PIPE2D-405 branch.

P.S. This is based on PIPE2D-442 (upgrade to LSST 18.1.0), because it requires functionality that doesn't exist in LSST 16.

cloomis can you review please?

Delay discussion of obs_pfs_data repo (raised during cloomis review) to a future ticket. Otherwise, review complete.

Merged to master.

And then discovered again that this requires LSST 18.1.0, so reverted the merge.

Merged to master (by reverting the revert of the accidental merge).