Repository: glue-viz/glue

Issue:
Allow WCSCoordinates to be initialized with only a WCS object
At the moment, WCSCoordinates **requires** a header, but we should change this so that a WCS object alone should be enough.

Discussion:

Patch:
diff --git a/glue/core/coordinates.py b/glue/core/coordinates.py
index 68b26e20..38001ce4 100644
--- a/glue/core/coordinates.py
+++ b/glue/core/coordinates.py
@@ -139,8 +139,8 @@ class Coordinates(object):
 
     def world_axis(self, data, axis):
         """
-        Find the world coordinates along a given dimension, and which for now we
-        center on the pixel origin.
+        Find the world coordinates along a given dimension, and which for now
+        we center on the pixel origin.
 
         Parameters
         ----------
@@ -152,10 +152,10 @@ class Coordinates(object):
 
         Notes
         -----
-        This method computes the axis values using pixel positions at the center
-        of the data along all other axes. This will therefore only give the
-        correct result for non-dependent axes (which can be checked using the
-        ``dependent_axes`` method)
+        This method computes the axis values using pixel positions at the
+        center of the data along all other axes. This will therefore only give
+        the correct result for non-dependent axes (which can be checked using
+        the ``dependent_axes`` method).
         """
         pixel = []
         for i, s in enumerate(data.shape):
@@ -163,7 +163,8 @@ class Coordinates(object):
                 pixel.append(np.arange(data.shape[axis]))
             else:
                 pixel.append(np.repeat((s - 1) / 2, data.shape[axis]))
-        return self.pixel2world_single_axis(*pixel[::-1], axis=data.ndim - 1 - axis)
+        return self.pixel2world_single_axis(*pixel[::-1],
+                                            axis=data.ndim - 1 - axis)
 
     def world_axis_unit(self, axis):
         """
@@ -173,7 +174,7 @@ class Coordinates(object):
         return ''
 
     def axis_label(self, axis):
-        return "World %i" % axis
+        return "World {}".format(axis)
 
     def dependent_axes(self, axis):
         """Return a tuple of which world-axes are non-indepndent
@@ -199,18 +200,31 @@ class WCSCoordinates(Coordinates):
     standard.  This class does not take into account
     distortions.
 
+    Parameters
+    ----------
+    header : :class:`astropy.io.fits.Header`
+        FITS header (derived from WCS if not given)
+    wcs : :class:`astropy.wcs.WCS`
+        WCS object to use, if different from header
+
     References
     ----------
-      * Greisen & Calabretta (2002), Astronomy and Astrophysics, 395, 1061
-      * Calabretta & Greisen (2002), Astronomy and Astrophysics, 395, 1077
-      * Greisen, Calabretta, Valdes & Allen (2006), Astronomy and
-        Astrophysics, 446, 747
+    * Greisen & Calabretta (2002), Astronomy and Astrophysics, 395, 1061
+    * Calabretta & Greisen (2002), Astronomy and Astrophysics, 395, 1077
+    * Greisen, Calabretta, Valdes & Allen (2006), Astronomy and
+      Astrophysics, 446, 747
     '''
 
-    def __init__(self, header, wcs=None):
+    def __init__(self, header=None, wcs=None):
         super(WCSCoordinates, self).__init__()
         from astropy.wcs import WCS
 
+        if header is None and wcs is None:
+            raise ValueError('Must provide either FITS header or WCS or both')
+
+        if header is None:
+            header = wcs.to_header()
+
         self._header = header
 
         try:
@@ -322,12 +336,17 @@ class WCSCoordinates(Coordinates):
 
 
 def coordinates_from_header(header):
-    """ Convert a FITS header into a glue Coordinates object
+    """
+    Convert a FITS header into a glue Coordinates object.
 
-    :param header: Header to convert
-    :type header: :class:`astropy.io.fits.Header`
+    Parameters
+    ----------
+    header : :class:`astropy.io.fits.Header`
+        Header to convert
 
-    :rtype: :class:`~glue.core.coordinates.Coordinates`
+    Returns
+    -------
+    coordinates : :class:`~glue.core.coordinates.Coordinates`
     """
 
     # We check whether the header contains at least CRVAL1 - if not, we would
@@ -340,12 +359,12 @@ def coordinates_from_header(header):
         try:
             return WCSCoordinates(header)
         except Exception as e:
-            logging.getLogger(__name__).warn("\n\n*******************************\n"
-                                             "Encounted an error during WCS parsing. "
-                                             "Discarding world coordinates! "
-                                             "\n%s\n"
-                                             "*******************************\n\n" % e
-                                             )
+            logging.getLogger(__name__).warn(
+                "\n\n*******************************\n"
+                "Encounted an error during WCS parsing. "
+                "Discarding world coordinates! "
+                "\n{}\n"
+                "*******************************\n\n".format(str(e)))
     return Coordinates()
 
 
@@ -358,10 +377,17 @@ def _get_ndim(header):
 
 
 def coordinates_from_wcs(wcs):
-    """Convert a wcs object into a glue Coordinates object
+    """
+    Convert an Astropy WCS object into a glue Coordinates object.
+
+    Parameters
+    ----------
+    wcs : :class:`astropy.wcs.WCS`
+        The WCS object to use
 
-    :param wcs: The WCS object to use
-    :rtype: :class:`~glue.core.coordinates.Coordinates`
+    Returns
+    -------
+    coordinates : :class:`~glue.core.coordinates.Coordinates`
     """
     from astropy.io import fits
     hdr_str = wcs.wcs.to_header()
@@ -375,7 +401,7 @@ def coordinates_from_wcs(wcs):
 
 def header_from_string(string):
     """
-    Convert a string to a FITS header
+    Convert a string to a FITS header.
     """
     from astropy.io import fits
     return fits.Header.fromstring(string, sep='\n')


Test patch:
diff --git a/glue/core/tests/test_coordinates.py b/glue/core/tests/test_coordinates.py
index 1e51f16d..232869df 100644
--- a/glue/core/tests/test_coordinates.py
+++ b/glue/core/tests/test_coordinates.py
@@ -4,7 +4,6 @@ from __future__ import absolute_import, division, print_function
 
 import pytest
 import numpy as np
-from mock import patch
 from numpy.testing import assert_allclose
 
 from glue.tests.helpers import requires_astropy
@@ -134,16 +133,12 @@ class TestWcsCoordinates(object):
         assert_allclose(result[1], expected[1])
 
     def test_pixel2world_invalid_input(self):
-        coord = WCSCoordinates(None)
-        x, y = {}, {}
-        with pytest.raises(TypeError) as exc:
-            coord.pixel2world(x, y)
+        with pytest.raises(ValueError):
+            WCSCoordinates(None)
 
     def test_world2pixel_invalid_input(self):
-        coord = WCSCoordinates(None)
-        x, y = {}, {}
-        with pytest.raises(TypeError) as exc:
-            coord.world2pixel(x, y)
+        with pytest.raises(ValueError):
+            WCSCoordinates(None)
 
     def test_axis_label(self):
         hdr = self.default_header()