-
Notifications
You must be signed in to change notification settings - Fork 14
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
convert varying_celestial_tranform schema ref to uri instead of tag #298
Conversation
Codecov ReportAll modified lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #298 +/- ##
=======================================
Coverage 98.28% 98.28%
=======================================
Files 40 40
Lines 2451 2451
=======================================
Hits 2409 2409
Misses 42 42 ☔ View full report in Codecov by Sentry. |
CHANGELOG.rst
Outdated
@@ -14,6 +14,7 @@ Bug Fixes | |||
- Fix minor bugs for header slicing functionality and expand test coverage for edge-cases. (`#275 <https://github.com/DKISTDC/dkist/pull/275>`_) | |||
- Fixed inverse transform in `.VaryingCelestialTransformSlit2D`. Which fixes a bug in VISP WCSes. (`#285 <https://github.com/DKISTDC/dkist/pull/285>`_) | |||
- Fix a bug preventing the transfer of a single dataset with :meth:`~dkist.net.transfer_complete_datasets`. (`#288 <https://github.com/DKISTDC/dkist/pull/288>`_) | |||
- Update varying celestial transform schema ref to use a uri instead of a tag.(`#298 <https://github.com/DKISTDC/dkist/pull/298>`_) |
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.
Same here as #297 could you add this to the changelog/
directory please?
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.
Thanks! I added 298.bugfix.rst. Should this be a 'bugfix' or something else?
@@ -30,7 +30,7 @@ properties: | |||
- tag: "core/ndarray-1.0.0" | |||
- tag: "tag:stsci.edu:asdf/unit/quantity-1.1.0" | |||
projection: | |||
$ref: "tag:stsci.edu:asdf/transform/transform-1.2.0" | |||
$ref: "http://stsci.edu/schemas/asdf/transform/transform-1.2.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.
Can this be / should this be asdf://
?
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.
…KISTDC#298) * convert varying_celestial_tranform schema ref to uri instead of tag * update changelog
asdf 3.0 will begin issuing a warning when a non-uri tag is encountered in a schema
$ref
. One such tag exists in the varying celestial transform schema:https://github.com/DKISTDC/dkist/blob/main/dkist/io/asdf/resources/schemas/varying_celestial_transform-1.0.0.yaml#L33
This PR updates the schema replacing the tag with the corresponding and equivalent uri (
http://stsci.edu/schemas/asdf/transform/transform-1.2.0
). The uri used to replace the tag is identical to the uri that is resolved by the tag and should have no impact on the schema or code that uses the schema (aside from preventing the warning when asdf 3.0 is released).