Providing a way to create and validate Solarnet-net compatible FITS headers #7271
Conversation
|
I'd love feedback and suggestions on this from a number of people including @hayesla, @DanRyanIrish, @ayshih, Amir Caspi, Dan Seaton, Andrew Robbertz @Alrobbertz. |
|
The rendered docs page can be found here. |
| Returns | ||
| ------- | ||
| result: bool | ||
| True if no issues were found. |
There was a problem hiding this comment.
Our validate() returns a list[str] of validation errors. I don't know if this is preferable compared to console warnings for errors and a single boolean for the validation check. You'd know better than I do how this would be integrated into other's code.
|
I'd like to use something like this at SDAC for understanding the degree of compliance present in the FITS files we get. The existence of this tool would also be useful for missions in their implementation phase. |
|
We did not talk about this at the AGU, shall we talk about this at the next weekly meeting? |
| WAVEMIN: | ||
| description: "[Angstrom] Minimum wavelength covered by filter" | ||
| human_readable: Minimum wavelength | ||
| required: true |
There was a problem hiding this comment.
It seems odd to me that this is required. This is a less-well defined quantity for some types of data, e.g. imaging instruments which have a bandpass or a magnetograph.
There was a problem hiding this comment.
The recommendations are specific to the kinds of observatories so you are right that this is technically only required for "Spectrographs and filter instruments
(Sections 3.2 and 5.4)" see page 59. I wasn't sure how to implement this kind of level of requirement system.
There was a problem hiding this comment.
I made this comment in the meeting discussion today, but I wonder whether we want to store this schema in sunpy itself. As this convention was developed elsewhere, I don't think we want to be the ones that own the spec/become de facto in charge of maintaining/updating the spec. My suggestion would be to either pull this schema from its official source or if that doesn't exist in a convenient format, then to put this in a separate, versioned repository that we can pull from such that the schema is maintained separately from sunpy.
There was a problem hiding this comment.
I think this is the right approach. Maybe @wafels could host an "official" solarnet repository given that their funding has ended? I'd like the same to be case for the CDF ISTP schema as well to be hosted by the SPDF.
There was a problem hiding this comment.
We could do this, although at the moment SDAC does not require compliance with the SOLARNET metadata standard. The heliophysics data policy does ask for FITS files for solar data, and the HEASARC hosts FITS verification tools. At the moment, testing against the schema would be purely optional for an instrument team.
There was a problem hiding this comment.
Was there a reason you recreated https://github.com/sunpy/sunpy/blob/7961e296893d52d2152c1f8ac0ba8f481813e28a/sunpy/map/header_helper.py in a new file?
There was a problem hiding this comment.
Yes, i moved it here because i created this new meta folder and thought it belonged here. Presumably it would become integrated with this schema system. Sorry I should have deleted the version where it currently lives.
| try: | ||
| u.Unit(card.value) | ||
| except ValueError: | ||
| warn_user(f'Value in {card.keyword} is not a recognized unit.') |
There was a problem hiding this comment.
If you want to make sure this adheres to the FITS standard, I think it needs to be a bit more strict, i.e.
Line 733 in 7961e29
| * Check that all required keywords are present. | ||
| * Check that keywords that have limited allowed values are consistent with those. | ||
| * Check that keyword values are consistent with the defined data types. | ||
| * Check that keywords that are quantities define the unit properly in the comment. | ||
| * Check that the comment are standard. | ||
| * Check that all keywords have values. |
There was a problem hiding this comment.
I think it would be better to separate out each of these steps into its own function and then have this method call each of these functions.
| # some special solarnet validations that cannot be included in the schema. | ||
| # the value of the keyword is a unit |
There was a problem hiding this comment.
Why can this not be included in the schema?
| deprecated_keywords = ['DATE-OBS', 'EXPTIME'] | ||
| replacement_keywords = ['DATE-BEG', 'XPOSURE'] |
There was a problem hiding this comment.
These should probably live as a property on the class and be a dictionary (key is the deprecated keyword, value is the replacement keyword).
sunpy/io/tests/test_fits_meta.py
Outdated
There was a problem hiding this comment.
Could you add a test that shows how/when/where this validation happens? Is this a separate function that a user runs on their FITS file? Would this validation happen when loading a Map?
There was a problem hiding this comment.
yes and yes. Just like astropy runs some validation when opening a FITS file I would suggest we do as well and potentially issue warnings when opening a Map (or any other FITS file).
Co-authored-by: Andrew Robbertz <24920994+Alrobbertz@users.noreply.github.com>
|
Thanks for the feedback @wtbarnes. @nabobalis looks like there is some interest in this functionality belong in SunPy, correct? @wafels and I need to discuss where the solarnet schema might live or be maintained but that can be figured out later. If we are in agreement on these points then I've asked @Alrobbertz to take over and developed this further so look out for a new PR from him in the near future. Please be nice! We are writing code in a new repo for the Space Weather SOC that will make use of this functionality and will therefore depend on SunPy for FITS file creation (which feels very appropriate). |
Correct. |
|
In fact, for my actual job, I will need this functionally. I have been asked to validate an entire FITS archive against the level 4 standard. So where do we go from here to get this ready for merging? |
This adds the ability to automatically create FITS headers that are compatible with the Solarnet recommendations. It provides a way to develop a schema for FITS headers which can be validated against.
I copied the existing functionality that existed in map/header_helper in this new module though I had not yet incorporated it.
This is a draft merge request with the following open tasks
The primary purpose of this functionality is to allow SunPy to validate headers with the soalrnet recommendations and enable new missions to easily create FITS file with correct headers. I personally plan on using this for PADRE/MeDDEA.