Add support for varying CRPIX in the VaryingTransform models for Cryo data #501
Conversation
Cadair
changed the title
| # for asdf standard < 1.6 | ||
| if tag.endswith("varying_celestial_transform-1.1.0"): | ||
| crpix_key = "crpix" |
There was a problem hiding this comment.
@braingram how terrible is this? lol
The newer tags have a minimum standard version of 1.6, so our oldest supported asdf writes the old tag.
| tags: | ||
| # the tag version does not match the schema version | ||
| - schema_uri: "asdf://dkist.nso.edu/schemas/varying_celestial_transform-1.2.0" | ||
| tag_uri: "asdf://dkist.nso.edu/tags/varying_celestial_transform-1.3.0" |
There was a problem hiding this comment.
Is there a reason these have different version numbers?
There was a problem hiding this comment.
Presumably because at some point in the past we bumped the tag and not the schema.
|
@Hyzerputt The downstream tests are failing on this PR, because we raise a deprecation warning on a renamed kwarg. This is an expected failure, but it brings up an interesting point about the workflow for deprecation warnings. They should make the tests fail because they shouldn't pass silently but they also shouldn't break unrelated things. Will having deprecation warnings fail integration tests and other runs which aren't the inventory unit tests? |
Mileage may vary but generally speaking, deprecation warnings don't fail the CI unit or integration tests. |
Related to: