DM-16297: Changes for Gen3 support of LSST data

[timj]

No description provided.

[timj]

I'm not really sure what to do about the changes to ingest.py. On the one hand it's clearly a bug in our headers that INHERIT=T is not set. On the other hand without this fix obs_lsst breaks. We don't want obs_lsst to have a special gen3 ingest task so what's the alternative?

[timj]

@czwa, @parejkoj this PR might be the most controversial bit of DM-16297. The problem is the forced merging of the primary header in ingest.py. I don't think we want to have special ingest code for gen3 instruments so this merging (which uses override mode so should be safe) is the general answer in the case where data has forgotten to set INHERIT=T (which is the case for some LSST data).

[TallJimbo]

I don't have much of a problem with special code for different Gen3 instruments, though it's obviously best minimized. The DM-17023 version of ingest already requires you to provide the fully-qualified Instrument class name via config parameter, so we have a place to hang such hooks (the registration system we had before was always a lie, as you had to import the module that defines that Instrument manually to make it work).

All that said, could we use fix_headers to add INHERIT=T instead?

[timj]

fix_headers runs after afwFits.readMetadata so that doesn't help. The code here to merge the headers doesn't actually harm anything since if INHERIT=T the merging would not change anything. The change is if INHERIT=F and that was deliberate.

If ingestion already has to know the Instrument class it is ingesting then I could add a new property to Instrument with a boolean indicating whether merging should happen or not (note that translate_header.py always merges because astropy.io.fits does not automatically follow INHERIT=T whereas afw does).

[TallJimbo]

To clarify, the only danger here is that some other camera's data intentionally has INHERIT missing (or INHERIT=F?), and this situation is that we just don't think that this is likely?

I don't think I'm particularly worried about that case, especially if we do the merging such that the extension header cards win when there are conflicts with the primary. I guess I don't really have a problem with either approach.

[timj]

Yes. The second header always overwrites the first header so what my change does is mean that we always merge even if INHERIT=F and that is only going to be a problem if the first header is completely unrelated to the first empty header. I'm okay with merging the headers now until we get to a point where an instrument turns up where that is bad.

[TallJimbo]

Looks good; only one minor comment beyond the (already converged) discussion above.