Using NDCube and NDMeta for GenericSpectrogram

[wtbarnes]

Caveat: Just putting up this PR to illustrate some of the ideas that came out of the Lorentz Center Workshop last week. Nothing works right now, this is not even a full implementation and it may never be. It's completely find if this particular PR is never merged, but the general idea should be pursued.

The basic idea here is to experiment with building GenericSpectrogram on top of NDCube rather than have it be its own object. Additionally, all of the derived metadata now lives on a separate metadata class. I've added some implementation notes, specifically separating out notes about the actual class, the metadata, and the factory. I've provided what I think is a reasonable path forward, but this likely needs more fleshing out.

@nabobalis please chime in if I missed anything here. I'm trying to reconstitute some of our conversations several days later.

See #209 for more context as to why we are pursuing this.

GenericSpectrogram

• Inherits from NDCube
• This subclass is intentionally minimal and adding any additional properties should be done very cautiously.
  • The intention is to eventually delete this subclass and just use NDCube once axis-specific properties are possible
  • To this end, adding methods to this subclass should also be avoided.
• Convenience properties are added for accessing the time and frequency arrays.
• Added a util function for building a gwcs from time and frequency arrays.
  • In principle, this WCS can be any APE-14-compliant WCS
  • Using a gwcs is likely easiest given that frequency is almost always spaced non-linearly and time can be as well.

Metadata

• All metadata properties have been moved to the .meta property of the NDCube
• The metadata is a SpectrogramMeta object which itself is a subclass of NDMeta
• It also inherits from the abstract base class SpectrogramMetaABC which itself is a subclass of MetaABC
• MetaABC enforces the existence of a number of metadata properties
• Compared to both the preexisting metadata properties on GenericSpectrogram and those identified during the workshop, the only property not already on MetaABC is target (the pointing coordinates of the observatory). This is why SpectogramMetaABC exists.
• SpectrogramMeta exists in order to inherit from SpectrogramMetaABC and to enable registration of source-specific metadata classes.
• Note that the WCS (the time/frequency axes) are not part of the metadata
• Polarization should be an extracoord

Factory

• The factory should continue to produce GenericSpectrogram instances
• In contrast to how the current factory works (and how the MapFactory works in sunpy), the factory should always produce GenericSpectrogram instances, but with source-specific metadata class instances attached.
• This is why the subclass registry now lives on SpectrogramMeta
• Trying to enforce a separation between data, WCS, and all other metadata
  • This is challenging because how the data and WCS are parsed is also source-specific and in principle, the metadata object does not know about the data or WCS properties.
  • So where does this source-specific parsing live?
  • The problem boils down to their not being a consistent file format across the many sources (unlike the sunpy Map/MapFactory which (essentially) only deals with FITS files)
• To some extent, there are really three different factories needed: data, WCS, and metadata, but we only want one.
  • We should be able to reduce this to one.

TODOs

A very rough, zeroth-order work plan. I'm not exactly sure the ordering here is ideal.

0. Review whether all of the needed metadata properties exist already on SpectrogramMeta. Add them to SpectrogramMetaABC if they don't already or if they can be derived from pre-existing metadata implement them as properties directly on SpectrogramMeta.
1. Implement the source-specific metadata classes.
   • These should all be SpectrogramMeta subclasses
   • To start with, it may be easiest to do all the file parsing on each source-specific class, even if their is code duplication across classes. Refactoring can be done later to reduce duplication.
   • It may also be best to parse the data and the time/frequency axes here as well and pass these back to the GenericSpectrogram constructor. This is awkward, but can be cleaned up later
2. Refactor the factory to use these metadata subclasses and produce GenericSpectrogram instances.
3. Refactor the plotting mixins to be similar to the NDCube plotting mixins.

[Amityush-lgtm]

Thanks for putting this together. One thing I wanted to ask before starting.
Should the new GenericSpectrogram still keep the old properties like times, frequencies, start_time and end_time working for now or should we start moving to the new time / frequency style directly?

[wtbarnes]

I'm more inclined toward time and frequency but I'll leave the decision up to the maintainers.

As far as time_start/end, I believe this is already covered in the metadata model imposed by MetaABC.

[hayesla]

see LC-radio-workshop/example_notebooks#11 too

[nabobalis]

This captures what I recall from our discussions.

The issues around the factory were the major pain points but a first working draft doesn't have to be pretty.

[samaloney]

Shouldn't forget about SpectralCoord and see how/if it works with GWCS as it already has observer and target properties.

Also look at NDCubeSequence e.g for multiple spectrograms from the same instrument/detector and
NDCollection e.g dealing with a collection of spectrograms from different instruemt/detector but with some overlap in in time