Update after initial review; remote tests working.

Author: weaverba137

astroquery/sdss/core.py

@@ -294,7 +294,8 @@ def query_region_async(self, coordinates, *, radius=None,
         ValueError
             If both ``radius`` and ``width`` are set (or neither),
             or if the ``radius`` exceeds 3 arcmin,
-            or if the sizes of ``coordinates`` and ``obj_names`` do not match.
+            or if the sizes of ``coordinates`` and ``obj_names`` do not match,
+            or if the units of ``width`` or ``height`` could not be parsed.
 
         Examples
         --------
@@ -322,9 +323,9 @@ def query_region_async(self, coordinates, *, radius=None,
             radius = 2.0 * u.arcsec
 
         if radius is None and width is None:
-            raise ValueError("Either radius or width must be selected!")
+            raise ValueError("Either radius or width must be specified!")
         if radius is not None and width is not None:
-            raise ValueError("Either radius or width must be selected!")
+            raise ValueError("Either radius or width must be specified, not both!")
 
         if radius is not None:
             request_payload, files = self.query_crossid_async(coordinates=coordinates,
@@ -339,25 +340,11 @@ def query_region_async(self, coordinates, *, radius=None,
                                                               data_release=data_release)
 
         if width is not None:
-            if isinstance(width, Angle):
-                width = width.to_value(u.degree)
-            else:
-                try:
-                    width = Angle(width).to_value(u.degree)
-                except ValueError:
-                    raise TypeError("width should be either Quantity or "
-                                    "convertible to float.")
+            width = u.Quantity(width, u.degree).value
             if height is None:
                 height = width
             else:
-                if isinstance(height, Angle):
-                    height = height.to_value(u.degree)
-                else:
-                    try:
-                        height = Angle(height).to_value(u.degree)
-                    except ValueError:
-                        raise TypeError("height should be either Quantity or "
-                                        "convertible to float.")
+                height = u.Quantity(height, u.degree).value
 
             dummy_payload = self._args_to_payload(coordinates=coordinates,
                                                   fields=fields,
@@ -378,7 +365,7 @@ def query_region_async(self, coordinates, *, radius=None,
                 coordinates = [coordinates]
 
             rectangles = list()
-            for n, target in enumerate(coordinates):
+            for target in coordinates:
                 # Query for a rectangle
                 target = commons.parse_coordinates(target).transform_to('fk5')
                 rectangles.append(self._rectangle_sql(target.ra.degree, target.dec.degree, width, height=height))

astroquery/sdss/tests/test_sdss.py

@@ -275,10 +275,14 @@ def test_sdss_photoobj(patch_request, dr):
 @pytest.mark.parametrize("radius", [None, Angle('2 arcsec')])
 @pytest.mark.parametrize("width", [None, Angle('2 arcsec')])
 def test_list_coordinates(patch_request, dr, radius, width):
-    if (radius is None and width is None) or (radius is not None and width is not None):
+    if (radius is None and width is None):
         with pytest.raises(ValueError) as e:
             sdss.SDSS.query_region(coords, radius=radius, width=width)
-        assert str(e.value) == "Either radius or width must be selected!"
+        assert str(e.value) == "Either radius or width must be specified!"
+    elif (radius is not None and width is not None):
+        with pytest.raises(ValueError) as e:
+            sdss.SDSS.query_region(coords, radius=radius, width=width)
+        assert str(e.value) == "Either radius or width must be specified, not both!"
     else:
         xid = sdss.SDSS.query_region(coords_list, radius=radius, width=width, data_release=dr)
 
@@ -298,16 +302,25 @@ def test_list_coordinates(patch_request, dr, radius, width):
                 url_tester(dr)
 
 
-@pytest.mark.parametrize("width", [Angle('2 arcsec'), 2.0 * u.arcsec, '2.0 arcsec', 'bad angle'])
-@pytest.mark.parametrize("height", [None, Angle('2 arcsec'), 2.0 * u.arcsec, '2.0 arcsec', 'bad angle'])
+@pytest.mark.parametrize("width", [Angle('2 arcsec'), 2.0 * u.arcsec, '2.0 arcsec', 'bad angle', '2.0 things'])
+@pytest.mark.parametrize("height", [None, Angle('2 arcsec'), 2.0 * u.arcsec, '2.0 arcsec', 'bad angle', '2.0 things'])
 def test_list_coordinates_with_height(patch_request, width, height):
-    if width == 'bad angle' or height == 'bad angle':
+    if width == 'bad angle':
         with pytest.raises(TypeError) as e:
             sdss.SDSS.query_region(coords, width=width, height=height)
-        if width == 'bad angle':
-            assert str(e.value) == "width should be either Quantity or convertible to float."
-        else:
-            assert str(e.value) == "height should be either Quantity or convertible to float."
+        assert str(e.value) == 'Cannot parse "bad angle" as a Quantity. It does not start with a number.'
+    elif width == '2.0 things':
+        with pytest.raises(ValueError) as e:
+            sdss.SDSS.query_region(coords, width=width, height=height)
+        assert str(e.value).startswith("'things' did not parse as unit")
+    elif height == 'bad angle':
+        with pytest.raises(TypeError) as e:
+            sdss.SDSS.query_region(coords, width=width, height=height)
+        assert str(e.value) == 'Cannot parse "bad angle" as a Quantity. It does not start with a number.'
+    elif height == '2.0 things':
+        with pytest.raises(ValueError) as e:
+            sdss.SDSS.query_region(coords, width=width, height=height)
+        assert str(e.value).startswith("'things' did not parse as unit")
     else:
         xid = sdss.SDSS.query_region(coords_list, width=width, height=height)
 

astroquery/sdss/tests/test_sdss_remote.py

@@ -3,6 +3,7 @@
 from numpy.testing import assert_allclose
 import pytest
 
+import astropy.units as u
 from astropy.coordinates import SkyCoord
 from astropy.table import Table
 from astropy.utils.exceptions import AstropyUserWarning
@@ -36,7 +37,7 @@ def test_images_timeout(self):
         This test *must* be run before `test_sdss_image` because that query
         caches!
         """
-        xid = sdss.SDSS.query_region(self.coords)
+        xid = sdss.SDSS.query_region(self.coords, width=2.0 * u.arcsec)
         assert len(xid) == 18
         try:
             with pytest.raises(TimeoutError):
@@ -51,9 +52,9 @@ def test_images_timeout(self):
     def test_sdss_spectrum(self, dr):
         if dr in dr_warn_list:
             with pytest.warns(AstropyUserWarning, match='Field info are not available for this data release'):
-                xid = sdss.SDSS.query_region(self.coords, spectro=True, data_release=dr)
+                xid = sdss.SDSS.query_region(self.coords, width=2.0 * u.arcsec, spectro=True, data_release=dr)
         else:
-            xid = sdss.SDSS.query_region(self.coords, spectro=True, data_release=dr)
+            xid = sdss.SDSS.query_region(self.coords, width=2.0 * u.arcsec, spectro=True, data_release=dr)
 
         assert isinstance(xid, Table)
         sdss.SDSS.get_spectra(matches=xid, data_release=dr)
@@ -85,7 +86,7 @@ class = 'galaxy'
         assert isinstance(xid, Table)
 
     def test_sdss_image(self):
-        xid = sdss.SDSS.query_region(self.coords)
+        xid = sdss.SDSS.query_region(self.coords, width=2.0 * u.arcsec)
         assert isinstance(xid, Table)
         sdss.SDSS.get_images(matches=xid)
 
@@ -170,14 +171,14 @@ def test_spectra_timeout(self):
 
     def test_query_non_default_field(self):
         # A regression test for #469
-        query1 = sdss.SDSS.query_region(self.coords, fields=['r', 'psfMag_r'])
+        query1 = sdss.SDSS.query_region(self.coords, width=2.0 * u.arcsec, fields=['r', 'psfMag_r'])
 
-        query2 = sdss.SDSS.query_region(self.coords, fields=['ra', 'dec', 'r'])
+        query2 = sdss.SDSS.query_region(self.coords, width=2.0 * u.arcsec, fields=['ra', 'dec', 'r'])
         assert isinstance(query1, Table)
         assert isinstance(query2, Table)
 
-        assert query1.colnames == ['objID', 'r', 'psfMag_r']
-        assert query2.colnames == ['objID', 'ra', 'dec', 'r']
+        assert query1.colnames == ['r', 'psfMag_r']
+        assert query2.colnames == ['ra', 'dec', 'r']
 
     @pytest.mark.parametrize("dr", dr_list)
     def test_query_crossid(self, dr):

docs/sdss/sdss.rst

@@ -28,7 +28,7 @@ photometry:
     >>> from astroquery.sdss import SDSS
     >>> from astropy import coordinates as coords
     >>> pos = coords.SkyCoord('0h8m05.63s +14d50m23.3s', frame='icrs')
-    >>> xid = SDSS.query_region(pos, spectro=True)
+    >>> xid = SDSS.query_region(pos, radius='5 arcsec', spectro=True)
     >>> print(xid)
            ra              dec        ...     specobjid      run2d
     ---------------- ---------------- ... ------------------ -----