-
Notifications
You must be signed in to change notification settings - Fork 22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
RCAL-919: Formalize the patches (sky cells) file (e.g., add it as a reference file) #524
base: main
Are you sure you want to change the base?
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #524 +/- ##
=======================================
Coverage 96.24% 96.24%
=======================================
Files 4 4
Lines 213 213
=======================================
Hits 205 205
Misses 8 8 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall LGTM modulo comments below about source
. I'm not up to date on the table format so don't feel informed enough to give this an approval. Let me know if you'd like me to study up on that to evaluate that portion of this PR.
description: | | ||
A structured array that contains relevant information about all the projection regions that | ||
cover the entire celestial sphere. | ||
source: 0 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source: 0 |
I'm not aware of any source
validator.
title: Information about all skycells | ||
description: | | ||
Relevant information about all skycells that have been defined to cover the celesital sphere. | ||
source: 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
source: 1 |
Same as above
$schema: asdf://stsci.edu/datamodels/roman/schemas/rad_schema-1.0.0 | ||
id: asdf://stsci.edu/datamodels/roman/schemas/reference_files/roman_skycells-1.0.0 | ||
|
||
title: Roman Skycells File Schema |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
title: Roman Skycells File Schema | |
title: Skycells File Schema |
While looking at the roman datamodels PR I noticed the roman prefixes there. Do we need these?
Same goes for the name of this file. Is skycells-1.0.0.yaml
sufficient?
ec7dc6b
to
e9d9c1b
Compare
for more information, see https://pre-commit.ci
Resolves RCAL
This PR addresses the need to support the skycell definition file as a reference file
The schema to support this file has been added to RAD.