Coordinate fixes

.gitignore

@@ -227,3 +227,4 @@ figure_test_images*
 
 # Release script
 .github_cache
+requirements-min.txt

changelog/305.breaking.rst

@@ -0,0 +1 @@
+We now require gwcs 0.19+ and therefore astropy 5.3+

changelog/305.bugfix.1.rst

@@ -0,0 +1 @@
+Fix the oversight that when generating a model for a celestial WCS the scale model was put before the affine transform in the pipeline. This means that the units for the affine transform matrix provided to ``VaryingCelestialTransform`` and ``generate_celestial_transform`` should be pixels not degrees.

changelog/305.bugfix.rst

@@ -0,0 +1 @@
+Fix missing references to parent transform schemas in ``Ravel`` and ``VaryingCelestialTransform`` ASDF schemas.

changelog/305.feature.rst

@@ -0,0 +1 @@
+Add a new ``AsymmetricMapping`` model to allow a different mapping in the forward and reverse directions.

changelog/305.internal.rst

@@ -0,0 +1 @@
+Remove all ``*Slit`` varying celestial transform models and replace them with a compound model combining a ``VaryingCelestialTransform`` and an ``AsymmetricMapping``.

dkist/conftest.py

@@ -179,7 +179,7 @@ def generate_lookup_table(lookup_table, interpolation='linear', points_unit=u.pi
 @pytest.fixture
 def identity_gwcs_5d_stokes(identity_gwcs_4d):
     stokes_frame = cf.StokesFrame(axes_order=(4,))
-    stokes_model = generate_lookup_table([0, 1, 2, 3] * u.one, interpolation='nearest')
+    stokes_model = generate_lookup_table([1, 2, 3, 4] * u.one, interpolation='nearest')
     transform = identity_gwcs_4d.forward_transform
     frame = cf.CompositeFrame(identity_gwcs_4d.output_frame.frames + [stokes_frame])
 

dkist/dataset/loader.py

@@ -155,7 +155,7 @@ def _load_from_asdf(filepath):
     try:
         with importlib_resources.as_file(importlib_resources.files("dkist.io") / "level_1_dataset_schema.yaml") as schema_path:
             with asdf.open(filepath, custom_schema=schema_path.as_posix(),
-                            lazy_load=False, copy_arrays=True) as ff:
+                           lazy_load=False, copy_arrays=True) as ff:
                 ds = ff.tree['dataset']
                 if isinstance(ds, TiledDataset):
                     for sub in ds.flat:

dkist/io/asdf/converters/__init__.py

@@ -1,4 +1,5 @@
 from .dataset import DatasetConverter
 from .file_manager import FileManagerConverter
-from .models import CoupledCompoundConverter, RavelConverter, VaryingCelestialConverter
+from .models import (AsymmetricMappingConverter, CoupledCompoundConverter,
+                     RavelConverter, VaryingCelestialConverter)
 from .tiled_dataset import TiledDatasetConverter

dkist/io/asdf/converters/dataset.py

@@ -19,6 +19,7 @@ def select_tag(self, obj, tags, ctx):
         return tags[0]
 
     def from_yaml_tree(self, node, tag, ctx):
+        tag_version = tuple(map(int, tag.split("-")[1].split(".")))
         from dkist.dataset import Dataset
 
         data = node["data"]._striped_external_array._generate_array()
@@ -27,6 +28,21 @@ def from_yaml_tree(self, node, tag, ctx):
         unit = node.get("unit")
         mask = node.get("mask")
 
+        # If we have a tag older than 1.2.0 then we are going to see if we can
+        # find a stokes table
+        if tag_version[0] == 0 or (tag_version[0] == 1 and tag_version[1] < 2):
+            # Put imports here to reduce import time on entry point load
+            import numpy as np
+
+            import astropy.units as u
+            from astropy.modeling.tabular import Tabular1D
+
+            # Find all the Tabular 1D models in the transform
+            for tabular in filter(lambda x: isinstance(x, Tabular1D), wcs.forward_transform.traverse_postorder()):
+                # Assert that the lookup table and points are what we expect for an old stokes table
+                if np.all(tabular.lookup_table == np.arange(4)) and u.allclose(tabular.points, np.arange(4)*u.pix):
+                    tabular.lookup_table = np.arange(1, 5) * u.one
+
         # Support older versions of the schema where headers was it's own top
         # level property
         if tag in ("tag:dkist.nso.edu:dkist/dataset-0.1.0",

dkist/io/asdf/converters/models.py

@@ -1,10 +1,15 @@
+import astropy.units as u
 from asdf_astropy.converters.transform.core import TransformConverterBase, parameter_to_value
 
 
 class VaryingCelestialConverter(TransformConverterBase):
     tags = [
+        "asdf://dkist.nso.edu/tags/varying_celestial_transform-1.1.0",
         "asdf://dkist.nso.edu/tags/varying_celestial_transform-1.0.0",
+        "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform-1.1.0",
         "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform-1.0.0",
+        # Old slit tags must be kept so we can read old files, but not types as
+        # we will not save slit classes any more
         "asdf://dkist.nso.edu/tags/varying_celestial_transform_slit-1.0.0",
         "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform_slit-1.0.0",
     ]
@@ -15,54 +20,29 @@ class VaryingCelestialConverter(TransformConverterBase):
         "dkist.wcs.models.InverseVaryingCelestialTransform2D",
         "dkist.wcs.models.VaryingCelestialTransform3D",
         "dkist.wcs.models.InverseVaryingCelestialTransform3D",
-        "dkist.wcs.models.VaryingCelestialTransformSlit",
-        "dkist.wcs.models.InverseVaryingCelestialTransformSlit",
-        "dkist.wcs.models.VaryingCelestialTransformSlit2D",
-        "dkist.wcs.models.InverseVaryingCelestialTransformSlit2D",
-        "dkist.wcs.models.VaryingCelestialTransformSlit3D",
-        "dkist.wcs.models.InverseVaryingCelestialTransformSlit3D",
     ]
 
     def select_tag(self, obj, tags, ctx):
         from dkist.wcs.models import (InverseVaryingCelestialTransform,
                                       InverseVaryingCelestialTransform2D,
                                       InverseVaryingCelestialTransform3D,
-                                      InverseVaryingCelestialTransformSlit,
-                                      InverseVaryingCelestialTransformSlit2D,
-                                      InverseVaryingCelestialTransformSlit3D,
                                       VaryingCelestialTransform, VaryingCelestialTransform2D,
-                                      VaryingCelestialTransform3D, VaryingCelestialTransformSlit,
-                                      VaryingCelestialTransformSlit2D,
-                                      VaryingCelestialTransformSlit3D)
+                                      VaryingCelestialTransform3D)
 
         if isinstance(
                 obj,
                 (VaryingCelestialTransform,
                  VaryingCelestialTransform2D,
                  VaryingCelestialTransform3D)
         ):
-            return "asdf://dkist.nso.edu/tags/varying_celestial_transform-1.0.0"
+            return "asdf://dkist.nso.edu/tags/varying_celestial_transform-1.1.0"
         elif isinstance(
                 obj,
                 (InverseVaryingCelestialTransform,
                  InverseVaryingCelestialTransform2D,
                  InverseVaryingCelestialTransform3D)
         ):
-            return "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform-1.0.0"
-        elif isinstance(
-                obj,
-                (VaryingCelestialTransformSlit,
-                 VaryingCelestialTransformSlit2D,
-                 VaryingCelestialTransformSlit3D)
-        ):
-            return "asdf://dkist.nso.edu/tags/varying_celestial_transform_slit-1.0.0"
-        elif isinstance(
-                obj,
-                (InverseVaryingCelestialTransformSlit,
-                 InverseVaryingCelestialTransformSlit2D,
-                 InverseVaryingCelestialTransformSlit3D)
-                 ):
-            return "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform_slit-1.0.0"
+            return "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform-1.1.0"
         else:
             raise ValueError(f"Unsupported object: {obj}")  # pragma: no cover
 
@@ -73,6 +53,18 @@ def from_yaml_tree_transform(self, node, tag, ctx):
         if "inverse_varying_celestial_transform" in tag:
             inverse = True
 
+        # Support reading files with the old Slit classes in them
+        slit = None
+        if "_slit" in tag:
+            slit = 1
+
+        # Old files (written with the 1.0.0 tag) have the wrong units attached
+        # to the pc_table (because of the incorrect ordering of the affine
+        # transform and scale models) so we change the units.
+        if tag.endswith("1.0.0"):
+            if node["pc_table"].unit is u.arcsec:
+                node["pc_table"] = u.Quantity(node["pc_table"].value, unit=u.pix)
+
         return varying_celestial_transform_from_tables(
             crpix=node["crpix"],
             cdelt=node["cdelt"],
@@ -81,7 +73,7 @@ def from_yaml_tree_transform(self, node, tag, ctx):
             pc_table=node["pc_table"],
             projection=node["projection"],
             inverse=inverse,
-            slit="_slit" in tag
+            slit=slit,
         )
 
     def to_yaml_tree_transform(self, model, tag, ctx):
@@ -161,3 +153,36 @@ def from_yaml_tree_transform(self, node, tag, ctx):
         from dkist.wcs.models import Ravel
 
         return Ravel(node["array_shape"], order=node["order"])
+
+
+class AsymmetricMappingConverter(TransformConverterBase):
+    """
+    ASDF serialization support for Ravel
+    """
+
+    tags  = [
+        "asdf://dkist.nso.edu/tags/asymmetric_mapping_model-1.0.0"
+    ]
+
+    types = ["dkist.wcs.models.AsymmetricMapping"]
+
+    def to_yaml_tree_transform(self, model, tag, ctx):
+        node = {
+            "forward_mapping": model.forward_mapping,
+            "backward_mapping": model.backward_mapping,
+        }
+        if model.forward_n_inputs is not None:
+            node["forward_n_inputs"] = model.forward_n_inputs
+        if model.backward_n_inputs is not None:
+            node["backward_n_inputs"] = model.backward_n_inputs
+        return node
+
+    def from_yaml_tree_transform(self, node, tag, ctx):
+        from dkist.wcs.models import AsymmetricMapping
+
+        return AsymmetricMapping(
+            node["forward_mapping"],
+            node["backward_mapping"],
+            node.get("forward_n_inputs"),
+            node.get("backward_n_inputs"),
+        )

dkist/io/asdf/entry_points.py

@@ -6,9 +6,9 @@
 from asdf.extension import ManifestExtension
 from asdf.resource import DirectoryResourceMapping
 
-from dkist.io.asdf.converters import (CoupledCompoundConverter, DatasetConverter,
-                                      FileManagerConverter, RavelConverter, TiledDatasetConverter,
-                                      VaryingCelestialConverter)
+from dkist.io.asdf.converters import (AsymmetricMappingConverter, CoupledCompoundConverter,
+                                      DatasetConverter, FileManagerConverter, RavelConverter,
+                                      TiledDatasetConverter, VaryingCelestialConverter)
 
 
 def get_resource_mappings():
@@ -37,12 +37,14 @@ def get_extensions():
     Get the list of extensions.
     """
     dkist_converters = [FileManagerConverter(), DatasetConverter(), TiledDatasetConverter()]
-    wcs_converters = [VaryingCelestialConverter(), CoupledCompoundConverter(), RavelConverter()]
+    wcs_converters = [VaryingCelestialConverter(), CoupledCompoundConverter(), RavelConverter(), AsymmetricMappingConverter()]
     return [
         ManifestExtension.from_uri("asdf://dkist.nso.edu/manifests/dkist-1.1.0",
                                    converters=dkist_converters),
         ManifestExtension.from_uri("asdf://dkist.nso.edu/manifests/dkist-1.0.0",
                                    converters=dkist_converters),
+        ManifestExtension.from_uri("asdf://dkist.nso.edu/manifests/dkist-wcs-1.2.0",
+                                   converters=wcs_converters),
         ManifestExtension.from_uri("asdf://dkist.nso.edu/manifests/dkist-wcs-1.1.0",
                                    converters=wcs_converters),
         ManifestExtension.from_uri("asdf://dkist.nso.edu/manifests/dkist-wcs-1.0.0",

dkist/io/asdf/resources/manifests/dkist-wcs-1.2.0.yaml

@@ -0,0 +1,18 @@
+%YAML 1.1
+---
+id: asdf://dkist.nso.edu/dkist/manifests/dkist-wcs-1.2.0
+extension_uri: asdf://dkist.nso.edu/dkist/extensions/dkist-wcs-1.2.0
+title: DKIST WCS extension
+description: ASDF schemas and tags for models and WCS related classes.
+
+tags:
+  - schema_uri: "asdf://dkist.nso.edu/schemas/varying_celestial_transform-1.0.0"
+    tag_uri: "asdf://dkist.nso.edu/tags/varying_celestial_transform-1.1.0"
+  - schema_uri: "asdf://dkist.nso.edu/schemas/varying_celestial_transform-1.0.0"
+    tag_uri: "asdf://dkist.nso.edu/tags/inverse_varying_celestial_transform-1.1.0"
+  - schema_uri: "asdf://dkist.nso.edu/schemas/coupled_compound_model-1.0.0"
+    tag_uri: "asdf://dkist.nso.edu/tags/coupled_compound_model-1.0.0"
+  - schema_uri: "asdf://dkist.nso.edu/schemas/ravel_model-1.0.0"
+    tag_uri: "asdf://dkist.nso.edu/tags/ravel_model-1.0.0"
+  - schema_uri: "asdf://dkist.nso.edu/schemas/asymmetric_mapping_model-1.0.0"
+    tag_uri: "asdf://dkist.nso.edu/tags/asymmetric_mapping_model-1.0.0"

dkist/io/asdf/resources/schemas/asymmetric_mapping_model-1.0.0.yaml

@@ -0,0 +1,30 @@
+%YAML 1.1
+---
+$schema: "http://stsci.edu/schemas/yaml-schema/draft-01"
+id: "asdf://dkist.nso.edu/schemas/asymmetric_mapping_model-1.0.0"
+title: >
+  Reorder, add and drop axes with different mappings in forward and reverse transforms.
+
+definitions:
+  mapping:
+    type: array
+    items:
+      type: integer
+
+allOf:
+  - $ref: "http://stsci.edu/schemas/asdf/transform/transform-1.2.0"
+  - properties:
+      forward_n_inputs:
+        description: |
+          Explicitly set the number of input axes in the forward direction.
+        type: integer
+      backward_n_inputs:
+        description: |
+          Explicitly set the number of input axes in the backward direction.
+        type: integer
+      forward_mapping:
+        $ref: "#/definitions/mapping"
+      backward_mapping:
+        $ref: "#/definitions/mapping"
+    required: [forward_mapping, backward_mapping]
+...

dkist/io/asdf/resources/schemas/ravel_model-1.0.0.yaml

@@ -7,10 +7,11 @@ title: A model to flatten 2D indices into 1D
 description:
   A model which takes a pair of indices and flattens them into the corresponding index for a 1D array. This can be used as a compound with Tabular1D to enable it to be indexed as if it were Tabular2D.
 
-type: object
-properties:
-  array_shape:
-    type: array
-  order:
-    type: string
-required: [array_shape, order]
+allOf:
+  - $ref: "http://stsci.edu/schemas/asdf/transform/transform-1.2.0"
+  - properties:
+      array_shape:
+        type: array
+      order:
+        type: string
+    required: [array_shape, order]

dkist/io/asdf/resources/schemas/varying_celestial_transform-1.0.0.yaml

@@ -7,31 +7,32 @@ title: A varying FITS-like celestial transform.
 description:
   A model which represents a FITS-like celestial WCS transform which varies over a third pixel input.
 
-type: object
-properties:
-  crpix:
-    anyOf:
-      - tag: "core/ndarray-1.*"
-      - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
-  cdelt:
-    anyOf:
-      - tag: "core/ndarray-1.*"
-      - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
-  lon_pole:
-    anyOf:
-      - type: number
-      - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
-  crval_table:
-    anyOf:
-      - tag: "core/ndarray-1.*"
-      - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
-  pc_table:
-    anyOf:
-      - tag: "core/ndarray-1.*"
-      - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
-  projection:
-    $ref: "http://stsci.edu/schemas/asdf/transform/transform-1.2.0"
+allOf:
+  - $ref: "http://stsci.edu/schemas/asdf/transform/transform-1.2.0"
+  - properties:
+      crpix:
+        anyOf:
+          - tag: "core/ndarray-1.*"
+          - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
+      cdelt:
+        anyOf:
+          - tag: "core/ndarray-1.*"
+          - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
+      lon_pole:
+        anyOf:
+          - type: number
+          - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
+      crval_table:
+        anyOf:
+          - tag: "core/ndarray-1.*"
+          - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
+      pc_table:
+        anyOf:
+          - tag: "core/ndarray-1.*"
+          - tag: "tag:stsci.edu:asdf/unit/quantity-1.*"
+      projection:
+        $ref: "http://stsci.edu/schemas/asdf/transform/transform-1.2.0"
 
-required: [crpix, cdelt, lon_pole, crval_table, pc_table, projection]
-additionalProperties: true
+    required: [crpix, cdelt, lon_pole, crval_table, pc_table, projection]
+    additionalProperties: true
 ...

dkist/io/asdf/tests/test_dataset.py

@@ -3,6 +3,7 @@
 
 import numpy as np
 import pytest
+from packaging.version import Version
 
 import asdf
 import astropy.table
@@ -171,3 +172,24 @@ def test_loader_getitem_with_chunksize(eit_dataset_asdf_path, wrap_object):
 
     expected_call = call((slice(0, chunksize[0], None), slice(0, chunksize[1], None)))
     assert expected_call in mocked.mock_calls
+
+
+def test_read_wcs_with_backwards_affine():
+    """
+    This test is a regression test for ASDF files with VCTs in them which were
+    written when we incorrectly put the scale before the affine transform in the
+    gWCS transforms.
+    """
+    # This test will only pass with the split axes fix in gwcs
+    dataset = dkist.load_dataset(rootdir / "test_old_wcs_BRMQY.asdf")
+    wcs = dataset.wcs
+    pixel_inputs = [0] * wcs.pixel_n_dim
+    world_outputs = wcs.pixel_to_world_values(*pixel_inputs)
+
+    # Assert that our stokes fixing code has worked.
+    assert world_outputs[-1] == 1
+
+    if Version(gwcs.__version__) > Version("0.20.dev0"):
+        pixel_outputs = wcs.world_to_pixel_values(*world_outputs)
+
+        assert np.allclose(pixel_inputs, pixel_outputs, atol=1e-6)