Apply suggestions from code review

asdf_astropy/converters/fits/fitswcs.py

@@ -2,29 +2,16 @@
 
 
 class FitsWCSConverter(Converter):
-    """
-    Converter for serializing and deserializing `astropy.wcs.WCS` objects.
-
-    This converter currently supports the serialization of simple WCS objects
-    by preserving the `wcs.to_header()` data. It does not support complex WCS objects
-    such as tabular or distortion WCSes.
-
-    Future work:
-        - Until the support for tabular and distortion WCSes is added,
-          throw error for such WCSes when passed through in the converter
-        - Implement mechanisms to detect tabular and distortion WCSes and support their serialization
-    """
-
     tags = ("tag:astropy.org:astropy/fits/fitswcs-*",)
     types = ("astropy.wcs.wcs.WCS",)
 
     def from_yaml_tree(self, node, tag, ctx):
         from astropy.wcs import WCS
 
-        header = node["header"]
-        return WCS(header)
+        primary_hdu = node["Hdu"][0]
+        return WCS(primary_hdu.header)
 
     def to_yaml_tree(self, wcs, tag, ctx):
         node = {}
-        node["header"] = dict(wcs.to_header())
+        node["Hdu"] = wcs.to_fits()
         return node

asdf_astropy/converters/fits/tests/test_fitswcs.py

@@ -1,42 +1,33 @@
+import warnings
+
 import asdf
 import pytest
-from astropy.wcs import WCS
+from astropy.io import fits
+from astropy.utils.data import download_file
+from astropy.wcs import WCS, FITSFixedWarning
 
 
 def create_wcs():
-    header = {
-        "CTYPE1": "TIME",
-        "CUNIT1": "min",
-        "CDELT1": 0.4,
-        "CRPIX1": 0,
-        "CRVAL1": 0,
-        "CTYPE2": "WAVE",
-        "CUNIT2": "Angstrom",
-        "CDELT2": 0.2,
-        "CRPIX2": 0,
-        "CRVAL2": 0,
-        "CTYPE3": "HPLT-TAN",
-        "CUNIT3": "arcsec",
-        "CDELT3": 20,
-        "CRPIX3": 0,
-        "CRVAL3": 0,
-        "CTYPE4": "HPLN-TAN",
-        "CUNIT4": "arcsec",
-        "CDELT4": 5,
-        "CRPIX4": 5,
-        "CRVAL4": 0,
-    }
-    return WCS(header)
+    urls = [
+        "http://data.astropy.org/tutorials/FITS-cubes/reduced_TAN_C14.fits",
+        "http://data.astropy.org/tutorials/FITS-images/HorseHead.fits",
+    ]
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore", FITSFixedWarning)
+        return [WCS(fits.open(download_file(url, cache=True))[0].header) for url in urls]
 
 
-@pytest.mark.parametrize("wcs", [create_wcs()])
+@pytest.mark.parametrize("wcs", create_wcs())
 def test_wcs_serialization(wcs, tmp_path):
     file_path = tmp_path / "test_wcs.asdf"
-    with asdf.AsdfFile() as af:
+    with asdf.AsdfFile() as af, warnings.catch_warnings():
+        warnings.simplefilter("ignore", FITSFixedWarning)
         af["wcs"] = wcs
         af.write_to(file_path)
 
-    with asdf.open(file_path) as af:
-        loaded_wcs = af["wcs"]
-
-    assert wcs.to_header() == loaded_wcs.to_header()
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore", FITSFixedWarning)
+        with asdf.open(file_path) as af:
+            loaded_wcs = af["wcs"]
+            assert wcs.to_header() == loaded_wcs.to_header()

asdf_astropy/converters/slicedwcs/slicedwcs.py

@@ -1,33 +1,34 @@
-import ast
-
 from asdf.extension import Converter
 
 
 class SlicedWCSConverter(Converter):
     tags = ("tag:astropy.org:astropy/slicedwcs/slicedwcs-*",)
     types = ("astropy.wcs.wcsapi.wrappers.sliced_wcs.SlicedLowLevelWCS",)
 
-    def parse_slice_string(self, slice_str):
-        if slice_str.isdigit():
-            return int(ast.literal_eval(slice_str))
-        slice_str = slice_str[len("slice(") : -1]
-        parts = slice_str.split(",")
-        start = ast.literal_eval(parts[0].strip())
-        stop = ast.literal_eval(parts[1].strip())
-        step = ast.literal_eval(parts[2].strip())
-        return slice(start, stop, step)
-
     def from_yaml_tree(self, node, tag, ctx):
         from astropy.wcs.wcsapi.wrappers.sliced_wcs import SlicedLowLevelWCS
 
         wcs = node["wcs"]
-        slice_array = node["slices_array"]
-        slice_array = [self.parse_slice_string(s) for s in slice_array]
-
+        slice_array = []
+        slice_array = [
+            s if isinstance(s, int) else slice(s["start"], s["stop"], s["step"]) for s in node["slices_array"]
+        ]
         return SlicedLowLevelWCS(wcs, slice_array)
 
     def to_yaml_tree(self, sl, tag, ctx):
         node = {}
         node["wcs"] = sl._wcs
-        node["slices_array"] = [str(item) for item in sl._slices_array]
+        node["slices_array"] = []
+
+        for s in sl._slices_array:
+            if isinstance(s, slice):
+                node["slices_array"].append(
+                    {
+                        "start": s.start if s.start is not None else None,
+                        "stop": s.stop if s.stop is not None else None,
+                        "step": s.step if s.step is not None else None,
+                    },
+                )
+            else:
+                node["slices_array"].append(s)
         return node

asdf_astropy/converters/slicedwcs/test/test_slicedwcs.py

@@ -1,7 +1,9 @@
+import warnings
+
 import asdf
 import numpy as np
 import pytest
-from astropy.wcs import WCS
+from astropy.wcs import WCS, FITSFixedWarning
 from astropy.wcs.wcsapi.wrappers.sliced_wcs import SlicedLowLevelWCS
 
 
@@ -24,11 +26,14 @@ def create_wcs():
 @pytest.mark.parametrize("sl_wcs", create_wcs())
 def test_sliced_wcs_serialization(sl_wcs, tmp_path):
     file_path = tmp_path / "test_slicedwcs.asdf"
-    with asdf.AsdfFile() as af:
+    with asdf.AsdfFile() as af, warnings.catch_warnings():
+        warnings.simplefilter("ignore", FITSFixedWarning)
         af["sl_wcs"] = sl_wcs
         af.write_to(file_path)
 
-    with asdf.open(file_path) as af:
-        loaded_sl_wcs = af["sl_wcs"]
-        assert sl_wcs._wcs.to_header() == loaded_sl_wcs._wcs.to_header()
-        assert sl_wcs._slices_array == loaded_sl_wcs._slices_array
+    with warnings.catch_warnings():
+        warnings.simplefilter("ignore", FITSFixedWarning)
+        with asdf.open(file_path) as af:
+            loaded_sl_wcs = af["sl_wcs"]
+            assert sl_wcs._wcs.to_header() == loaded_sl_wcs._wcs.to_header()
+            assert sl_wcs._slices_array == loaded_sl_wcs._slices_array

asdf_astropy/resources/manifests/astropy-1.0.0.yaml

@@ -28,16 +28,6 @@ tags:
     be accessible in its proper form in the ASDF file.
 
     Only image and binary table extensions are supported.
-- tag_uri: tag:astropy.org:astropy/fits/fitswcs-1.0.0
-  schema_uri: http://astropy.org/schemas/astropy/fits/fitswcs-1.0.0
-  title: Represents an astropy.wcs.WCS object
-  description: |-
-    Supports serialization of the simple wcs objects header
-- tag_uri: tag:astropy.org:astropy/slicedwcs/slicedwcs-1.0.0
-  schema_uri: http://astropy.org/schemas/astropy/slicedwcs/slicedwcs-1.0.0
-  title: Represents a SlicedLowLevelWCS object
-  description: |-
-    Supports serialization of the sliced_wcs objects
 - tag_uri: tag:astropy.org:astropy/table/table-1.0.0
   schema_uri: http://astropy.org/schemas/astropy/table/table-1.0.0
   title: A table.

asdf_astropy/resources/manifests/astropy-1.1.0.yaml

@@ -54,3 +54,20 @@ tags:
   title: NdarrayMixin column.
   description: |-
     Represents an astropy.table.NdarrayMixin instance.
+- tag_uri: tag:astropy.org:astropy/slicedwcs/slicedwcs-1.0.0
+  schema_uri: http://astropy.org/schemas/astropy/slicedwcs/slicedwcs-1.0.0
+  title: Represents an instance of SlicedLowLevelWCS
+  description: |-
+    The SlicedLowLevelWCS class is a wrapper class for WCS that applies slices
+    to the WCS, allowing certain pixel and world dimensions to be retained or
+    dropped.
+
+    It manages the slicing and coordinate transformations while preserving
+    the underlying WCS object.
+- tag_uri: tag:astropy.org:astropy/fits/fitswcs-1.0.0
+  schema_uri: http://astropy.org/schemas/astropy/fits/fitswcs-1.0.0
+  title: FITS WCS (World Coordinate System) Converter
+  description: |-
+    Represents the FITS WCS object, the HDUlist of the FITS header is preserved
+    during serialization and during deserialization the WCS object is recreated
+    from the HDUlist.

asdf_astropy/resources/schemas/fits/fitswcs-1.0.0.yaml

@@ -10,8 +10,7 @@ description:
   Represents the fits object
 
 allOf:
-  - tag: "tag:astropy.org:astropy/fits/fitswcs-1.0.0"
   - type: object
     properties:
-      header:
-        type: object
+      hdu:
+        tag: "tag:stsci.edu:asdf/fits/fits-*"

asdf_astropy/resources/schemas/slicedwcs/slicedwcs-1.0.0.yaml

@@ -17,3 +17,9 @@ allOf:
         tag: "tag:astropy.org:astropy/fits/fitswcs-1.0.0"
       slices_array:
         type: array
+        items:
+          - oneOf:
+            - type: integer
+            - type: object
+
+    required: ["wcs", "slices_array"]