Defining a Standardised Metadata Attribute API #61
Conversation
There was a problem hiding this comment.
General comments:
- This would be much more useful as a standard if the return types of the properties were specified. This would be easy to do by adding type hints.
- Several of these properties are redundant in the sense that they can be calculated from a combination of other properties. Do these really belong in this standard?
| Coordinate of observatory location. | ||
| """ | ||
|
|
||
| def observer_radial_velocity(self): |
There was a problem hiding this comment.
If observer_location is a SkyCoord, can't the velocity be bundled in there? (see https://docs.astropy.org/en/latest/coordinates/velocities.html for more technical info)
There was a problem hiding this comment.
I like this suggestion :)
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def celestial_frame(self): |
There was a problem hiding this comment.
What does this mean? (I think this is a case where typing the return would help!) Is it the WCS? Or part of the WCS?
There was a problem hiding this comment.
This is trying to play the role of Map.coordinate_frame. The reason I called it celestial_frame was we don't know what the physical types of the associated data, unlike with Map.
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def distance_to_sun(self): |
There was a problem hiding this comment.
This is redundant if you specify the observer location and observation time.
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def light_travel_time(self): |
There was a problem hiding this comment.
This is poorly defined - for an image of the Sun for example the light travel time will be different across the image detector because the limb of the Sun is further away than disc center.
There was a problem hiding this comment.
This is certainly true for large fields of view. But in smaller fields of view or higher energy images where only a flare emits sufficiently to be detected it is well (or better) defined.
Having said that though, there is nothing preventing this returning a Quantity with the same shape as the spatial axes giving the light travel time for each pixel. The other Metadata PR open in this repo describing a Sliceable Metadata object would support this.
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def light_travel_time_to_earth(self): |
There was a problem hiding this comment.
Again, this is redundant if you specify the observer coordinate and observation time.
There was a problem hiding this comment.
This is true. But I imagine this would still be a useful convenience property that does the calculation for the user instead of requiring them to type the couple lines of code every time.
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def date_average(self): |
There was a problem hiding this comment.
Isn't this redundant if you have the start/end time? If not, how exactly is mean/representative defined?
There was a problem hiding this comment.
For this, think DATE_OBS in a FITS header. It's "the" time of the observation, however that's defined by the instrument team.
However, if the data is in an NDCube I would argue this information should be stored in .global_coords. So if we link this metadata standard with NDCube I think this property should be removed. But here I haven't required that this metadata class be used with an NDCube instance. Perhaps we should?
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def exposure_time(self): |
There was a problem hiding this comment.
Redundant given start/end time?
There was a problem hiding this comment.
This isn't necessarily true. The start/end time can represent the cadence. Moreover, if there are multiple images/measurements over time, the start/end times will represent the observing duration, not the exposure time of each image.
| @abc.abstractproperty | ||
| def rsun_meters(self): | ||
| """ | ||
| Solar radius in units of length. |
There was a problem hiding this comment.
| Solar radius in units of length. | |
| Assumed radius of emmission measured from the solar center. |
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def rsun_angular(self): |
There was a problem hiding this comment.
Redundant given observer coordinate and rsun_meters?
There was a problem hiding this comment.
True. But again in that case, this is still a useful convenience property. Moreover who's to say that rsun_meters is provided in the header. Perhaps only rsun_angular is.
| """ | ||
|
|
||
| @abc.abstractproperty | ||
| def SAA(self): |
There was a problem hiding this comment.
Redundant given observer position/time?
There was a problem hiding this comment.
An SAA flag is often provided in FITS headers. So there is a philosophical question here. Is this metadata object for the minimal set of metadata from which all others can be derived? Or is it to represent the metadata attached to the data? Put another way, do we discard redundant information provided by instrument teams and ask users to recalculate it from other metadata? If we do, who's to say these will provide the same results? Where is the boundary of the SAA? It may depend on the instrument's sensitivity to the SAA. In this case a general user may come up with a different result to the more knowledgeable instrument team.
|
I would strongly encourage interoperability with the SPASE Data Model to pull in the broadest number of use cases. |
Good suggestion!
I think my reply to your comment on the The first option is cleaner but assumes that all the minimal metadata elements are supplied which may not be true. It also assumes that all redundant metadata is self-consistent, which again may not be true perhaps for valid reasons. The second option is messier but makes fewer assumptions of the "quality" of the metadata. |
Thanks for the suggestion @MSKirk. Could you elaborate on what this means in practice? At first glance the only labels I see that overlap are |
Those are the different resource types. The top level entity in the SPASE data model is a Resource. There are 12 different types of resources. Each resource type consists of a set of attributes that characterize the resource.The resource types can be divided into three categories: Data Resources, Origination Resources and Infrastructure Resources. See Section 3.1.4 for a graphical description. Under each of these resources, there is an entire set of meta data elements that describe all of the essential elements of the data product. Check out Section 8 for an overview of each of these elements. |
PR Description
This SEP attempts to define standardised attribute names via which common solar metadata can be accessed via a future metadata class.
Please provide feedback on whether the tings I've included should be there, whether other things should be added, and whether the names I've chosen are the right choice.