Modifying our approach to Map metadata

[ayshih]

The current storage of our Map metadata (.meta) is confusing because it is the combination of:

• keywords in the original header that we trust and use actively for calculations
• keywords in the original header that we know have wrong values (e.g., SUVI FITS headers have some inaccurate observer information #3906, HMI FITS headers may not have accurate observer coordinates #3057, AIA FITS headers have inaccurate HGS coordinates #3038)
• keywords in the original header that we have no opinion on
• keywords that we've overwritten from the original header
• keywords that we've added compared to the original header (e.g., PCi_j)
• keywords that we've removed compared to the original header (e.g., CROTA2)

While we our API does have properties as an interface to .meta, it's unavoidable for users to directly access .meta, and then it's opaque to the user whether they can trust what's in .meta.

I suggest a modified approach to Map metadata. Rather than having a single dictionary .meta, we would have two dictionaries:

• .original_header would preserve the original values from the header, and we don't modify this header at all after it is created
• .meta would contain only those keywords that we "bless", whether they are copied from .original_header or are generated through our own code

For example, if there's a PCi_j keyword in .original_header, we'd simply copy that to .meta. If there's only CROTA2, we'd calculate the appropriate PCi_j keywords and put them in .meta, but we wouldn't modify .original_header. The CROTA2 keyword would never show up in .meta.

For observer coordinates, we'd copy over to .meta only the set of coordinates that we trust from .original_header. In the unfortunate cases where there are no correct values (e.g., #3057, #3037), we could choose to copy over nothing to .meta so that it's explicit that there are no "blessed" values. Even if we automatically determine the observer location through external sources (e.g., JPL Horizons), we'd put those "blessed" values in .meta but still not touch .original_header.

It's an open question what we save when saving to a file. I haven't thought much about the considerations there.

Thoughts?

[dstansby]

I'm 👍 for saving the original 100% untouched header somewhere. Whilst writing map classes for synoptic maps it is often helpful to validity check metadata fixes against parts of the original header that may be subsequently modified by the map class, so keeping a copy of the original header would be very useful in at least that case.

[ayshih]

I'll preserve here some Riot discussion between myself and @Cadair:

@Cadair:
but my thinking was we would have something quite a lot like that in some kinda metadata object

@ayshih:
We've been talking about a metadata object for years, and it feels about as real as fusion power plants. I think a two-dictionary solution would be fairly straightforward to implement, organizes the existing mess, and may even uncover some bugs or faulty assumptions (e.g., accidentally using non-trusted values).

@Cadair:
yeah that's fair. I am reluctant to do too much that isn't NDCube compat at this point though. Two dicts in a wrapper object would be fine as a first pass.

[Cadair]

I am going to maintain my position that this is worth doing properly in the 3.x cycle. It's rattling around my brain at the moment and I might take a swing at putting together a concrete proposal for better metadata handling.

[nabobalis]

Do we want to SEP this?

[Cadair]

Do we want to SEP this?

probably.

Also there are probably too many things rattling around in my brain for me to actually get to taking a swing at this if someone else wants to. @DanRyanIrish 😁

[Cadair]

I am going to pull some more recent thoughts of mine into this issue. A bunch of these have come from #5199.

Rather than splitting the header into two sets: meta and original_header I propose we instead split them into three things:

• A collection of Python objects representing known, extracted metadata. Things like a WCS, a SkyCoord for the observer location etc. Any keys in the input header which are used to construct these objects would be removed.
• All the other unknown / unloved metadata, which as soon as you call a method like rotate may become invalid.
• The original header as loaded out of the file; for reference.

The second and third parts, could arguably be dropped when a new instance of the map is made (i.e. rotate).

I think it's important to opening up future functionality that we move away from a FITS header-like object as the sole truth of map's metadata model and start using actual Python objects for that. Only if we do this will we be able to support objects like gWCS and other file formats like asdf.