Add spectrogram metadata classes by Amityush-lgtm · Pull Request #245 · sunpy/radiospectra

Amityush-lgtm · 2026-06-27T13:39:41Z

PR Description

Fixes #243

Implementing a metadata class hierarchy for radiospectra, following the pattern used in sunraster and the LC Radio Workshop metadata requirements.
I tried to make some flowcharts as well, will attach them too.

Changes:

Added SpectrogramMetaABC and SpectrogramMeta (backed by NDMeta) in radiospectra/spectrogram/meta.py
Added instrument specific subclasses (WAVESMeta, SWAVESMeta, CALISTOMeta, EOVSAMeta, RSTNMeta)
Updated GenericSpectrogram to wrap raw dict metadata into SpectrogramMeta automatically
Added tests for the new meta class

AI Assistance Disclosure

AI tools were used for:

Code generation (e.g., when writing an implementation or fixing a bug)
Test/benchmark generation
Documentation (including examples)
Research and understanding
No AI tools were used

Regardless of AI use, the human contributor remains fully responsible for correctness, design choices, licensing compatibility, and long-term maintainability.

This is an initial draft, so I've just tried to make the changes in WAVES and CALISTO, if it looks in the right direction then I'll add the remaining subclasses also. Let me know your thoughts on this @samaloney and @hayesla.

Amityush-lgtm · 2026-06-29T08:06:00Z

Sharing the flowchart i used while designing this:

samaloney · 2026-06-29T12:49:44Z

+    # Identification
+    @property
+    @abc.abstractmethod
+    def instrument(self):


I think a short description and the type info should be encode here for each property e.g.

def instrument(self) -> str: """ Name of the instrument """ pass

and then here if we only return str then the default south be "" if that better or worse then returning a str or none e.g. -> str|None

for properties like date_start, frequency_rangeor data_units, what should i write for them??

"Start of the observation" Time
"Frequency range of observation" Quantity
"Unit for the data" Unit

Maybe we don't need the docstrings for them all but they types would be good

One more doubt, what would you prefer for observer_location and data_mask?

samaloney · 2026-07-01T08:58:07Z

Also you can embed mermaid charts directly in GitHub doesn't match exactly the UML but close enough

classDiagram
    direction TB

    class NDMetaABC {
        <<ndcube.meta>>
    }

    class SpectrogramMetaABC {
        <<abstract>>
        +instrument
        +observatory
        +detector
        +date_start
        +date_end
        ...
    }

    class NDMeta {
        <<ndcube.meta>>
        dict subclass
        +axes
        ...
        +slice()
        +add()
        +rebin()
    }

    class SpectrogramMeta {
        <<concrete base>>
        dict-backed via NDMeta
        +instrument
        +observatory
        +detector
        +date_start
        +date_end
        ...
    }

    class WAVESMeta {
        +background
        +receiver
    }

    class CALISTOMeta {
        +observer_location
    }

    class OtherInstrumentMetas {
        <<...>>
    }

    %% Relationships
    SpectrogramMetaABC --|> NDMetaABC : inherits
    NDMeta --|> NDMetaABC : inherits

    SpectrogramMeta --|> NDMeta : inherits
    SpectrogramMeta ..|> SpectrogramMetaABC : implements

    WAVESMeta --|> SpectrogramMeta
    CALISTOMeta --|> SpectrogramMeta
    OtherInstrumentMetas --|> SpectrogramMeta

samaloney · 2026-07-01T08:59:43Z

+    @property
+    @abc.abstractmethod
+    def data_units(self) -> Unit:
+        """Unit for the data"""
+        pass


I think this is aleady cover by the unit in NDCube

samaloney · 2026-07-01T09:00:57Z

+    def observer_location(self):
+        """Observer location."""
+        pass


Maybe observe_coordinate would be better and this should be a SkyCoord | None

Got it, I'm working on it

Amityush-lgtm · 2026-07-01T11:15:13Z

Also you can embed mermaid charts directly in GitHub doesn't match exactly the UML but close enough

classDiagram
    direction TB

    class NDMetaABC {
        <<ndcube.meta>>
    }

    class SpectrogramMetaABC {
        <<abstract>>
        +instrument
        +observatory
        +detector
        +date_start
        +date_end
        ...
    }

    class NDMeta {
        <<ndcube.meta>>
        dict subclass
        +axes
        ...
        +slice()
        +add()
        +rebin()
    }

    class SpectrogramMeta {
        <<concrete base>>
        dict-backed via NDMeta
        +instrument
        +observatory
        +detector
        +date_start
        +date_end
        ...
    }

    class WAVESMeta {
        +background
        +receiver
    }

    class CALISTOMeta {
        +observer_location
    }

    class OtherInstrumentMetas {
        <<...>>
    }

    %% Relationships
    SpectrogramMetaABC --|> NDMetaABC : inherits
    NDMeta --|> NDMetaABC : inherits

    SpectrogramMeta --|> NDMeta : inherits
    SpectrogramMeta ..|> SpectrogramMetaABC : implements

    WAVESMeta --|> SpectrogramMeta
    CALISTOMeta --|> SpectrogramMeta
    OtherInstrumentMetas --|> SpectrogramMeta

Thats so cool, I didn't know GitHub supported Mermaid diagrams. I'll definitely try using them, Thanks a lot!

Amityush-lgtm · 2026-07-02T18:54:30Z

Should i add the rest of the classes in this PR or like break it in different PRs??

samaloney · 2026-07-02T19:20:43Z

I think adding the classes necessary to handle the instruments that are currently supported would make sense.

Amityush-lgtm · 2026-07-03T19:06:18Z

I have added the remaining classes, let know your thoughts on this @samaloney

hayesla

Thanks for this @Amityush-lgtm! this is looking good (sorry for delay)

I think there are some issue here, I think it would also be good to make some tests making some generic wrapping itself, for example a test that constructs a GenericSpectrogram from a dict and assert the meta etc, and that the things like spec.observatory/start_time/etc return the right types?

it is probably also good to check that meta survives slicing (i.e. if you slice the Spectrogram does the meta slice?), somethign worth checking! this may also be good to do in another PR

hayesla · 2026-07-13T07:11:25Z

+        return self.get("frequency_resolution")
+
+    @property
+    def calibration_state(self) -> str | None:


hmm I wonder if calibration_state is the right word here? could it just be calibrated and then have boolean? @samaloney would there ever be a case where the data is partially calibrated?

🤷 I'm not sure if this isn't redundant with level but maybe that's ok?

hayesla · 2026-07-13T07:12:44Z

+    """Metadata for WIND/WAVES spectrograms."""
+
+    @property
+    def background(self):


why is this here? only on WAVES?

background is only used by WAVES, i added it on WAVESMeta to make it easy to find. Should i move it in the base class ??

hayesla · 2026-07-13T07:14:12Z

+    """Metadata for PSP FIELDS/RFS spectrograms."""
+
+    @property
+    def level(self) -> str | None:


this is the processing_level no?

yes, it is processing_level but since it will override from the base class, thats why i changed its name. I've updated it to override.

So this is the point is we map different convention e.g. name back to our "standard" name.

hayesla · 2026-07-13T07:55:08Z

        """
        The name of the observatory which recorded the spectrogram.
        """
-        return self.meta["observatory"].upper()
+        val = getattr(self.meta, "observatory", self.meta.get("observatory"))
+        return val.upper() if val else None


hmm there's an issue with this, this getattr finds the observatory property on SpectrogramMeta, and so the .get() fallback here is never used - so there is actually a bug with a KeyError being raised is observatory isnt passed

I've tried to simplify these to just use self.meta.get() directly for fallback

hayesla · 2026-07-13T08:00:00Z

+    """Metadata for e-CALLISTO spectrograms."""
+
+    @property
+    def observer_coordinate(self) -> SkyCoord | None:


this actually returns an EarthLocation not a SkyCoord - i think we should actually use a SkyCoord (i.e. convert the EarthLocation to a SkyCoord) i think this can be done with .get_gcrs(obstime)

hayesla · 2026-07-13T08:06:21Z

+
+    @property
+    def date_start(self) -> Time:
+        return self["start_time"]


should this be

Suggested change

return self["start_time"]

return Time(self["start_time"])

```?

like the -> Time annotation only holds if the caller stored a Time?

hayesla · 2026-07-13T08:08:06Z

+        return self["start_time"]
+
+    @property
+    def date_end(self) -> Time:


similarly here

hayesla · 2026-07-13T08:11:24Z

+        {
+            "processing_level": "L2",
+            "version": "1.0",
+            "wavelength": "radio",


this wavelength here shouldnt be a string? (i realise this is just a test but its not right)

hayesla · 2026-07-13T08:12:15Z

+        return self.get("background")
+
+    @property
+    def receiver(self):


is this specific to waves? it could be on base class?

hayesla · 2026-07-13T08:12:36Z

        super().__init__(meta=meta, data=data, **kwargs)

    @property
    def receiver(self):


see comment below about this maybe being on base class

…rocessing_level

Amityush-lgtm · 2026-07-14T06:50:37Z

I've pushed the latest changes with all the suggested updates, let me know if i've missed anything or anything else to improve

Amityush-lgtm added 3 commits June 27, 2026 18:54

Initial draft of SpectrogramMeta classes

23be630

Adding changelog

0d3fb5e

cleaning up comments and callisto location logic

132e1a1

samaloney reviewed Jun 29, 2026

View reviewed changes

Amityush-lgtm added 2 commits July 1, 2026 04:36

adding type info and docstrings

b519dc8

adding type info and docstrings in some more places

104f8d2

samaloney reviewed Jul 1, 2026

View reviewed changes

Removinh data_units and rename observer_location to observer_coordinate

6672094

add meta tests for CALISTO and WAVES

357ff3c

add meta-classes for remaining instruments

0ce6a01

Amityush-lgtm marked this pull request as ready for review July 3, 2026 19:03

Amityush-lgtm requested a review from samaloney July 3, 2026 19:06

Updating changelog

b9ea4d8

hayesla reviewed Jul 13, 2026

View reviewed changes

Amityush-lgtm added 4 commits July 14, 2026 02:29

fix base metadata bug and property type hints

eb8cc83

Update callisto observer_coordinate to SkyCoord and rename level to p…

f590ce8

…rocessing_level

Add generic spectrogram test

7eba11a

Moving receiver and background to base and using Time on dates

065e540

Amityush-lgtm requested a review from hayesla July 14, 2026 07:14

Amityush-lgtm changed the title ~~Initial draft of SpectrogramMeta classes~~ Add spectrogram metadata classes Jul 17, 2026

hayesla added the NDCube Refactor This label is for issues/PR related to NDCube refactor for GSOC project 2026 label Jul 23, 2026

samaloney mentioned this pull request Jul 24, 2026

Define the min/required API a Spectrum meta data object sunpy/sunkit-spex#290

Open

	return self["start_time"]
	return Time(self["start_time"])
	```?

Uh oh!

Uh oh!

Conversation

Amityush-lgtm commented Jun 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Description

AI Assistance Disclosure

Uh oh!

Amityush-lgtm commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

samaloney Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

samaloney commented Jul 1, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amityush-lgtm Jul 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amityush-lgtm commented Jul 1, 2026

Uh oh!

Amityush-lgtm commented Jul 2, 2026

Uh oh!

samaloney commented Jul 2, 2026

Uh oh!

Amityush-lgtm commented Jul 3, 2026

Uh oh!

hayesla left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amityush-lgtm Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amityush-lgtm Jul 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Amityush-lgtm commented Jul 14, 2026

Uh oh!

Amityush-lgtm commented Jun 27, 2026 •

edited

Loading

Amityush-lgtm commented Jun 29, 2026 •

edited

Loading

samaloney Jun 29, 2026 •

edited

Loading

Amityush-lgtm Jul 1, 2026 •

edited

Loading

Amityush-lgtm Jul 13, 2026 •

edited

Loading

Amityush-lgtm Jul 13, 2026 •

edited

Loading