Merge pull request #1308 from weaverba137/update-sdss-iv

[WIP] Update SDSS module to SDSS-IV URLs

Author: keflavich

.travis.yml

@@ -98,13 +98,13 @@ matrix:
         # Test the oldest astropy version without optional dependencies.
         - os: linux
           stage: Tests with other Python/Numpy versions
-          env: PYTHON_VERSION=3.4 NUMPY_VERSION=1.10  KEYRING_VERSION='<12.0'
+          env: PYTHON_VERSION=3.5 NUMPY_VERSION=1.12  KEYRING_VERSION='<12.0'
                CONDA_DEPENDENCIES="keyring requests beautifulsoup4 html5lib secretstorage"
                SECRETSTORAGE_VERSION='<3.0.1'
-               ASTROPY_VERSION=1.3 PYTEST_VERSION='3.1' ASTROPY_USE_SYSTEM_PYTEST=1
+               ASTROPY_VERSION=2 PYTEST_VERSION='<3.7'
         - os: linux
           stage: Tests with other Python/Numpy versions
-          env: PYTHON_VERSION=3.5.5 NUMPY_VERSION=1.11 KEYRING_VERSION='<12.0'
+          env: PYTHON_VERSION=3.5.5 NUMPY_VERSION=1.12 KEYRING_VERSION='<12.0'
                ASTROPY_VERSION=3.0
 
         - os: linux

CHANGES

@@ -5,6 +5,7 @@
 - ADS: Fix an error in one of the default keys, citations->citation [#1337]
 - HEASARC: Fixing error handling to filter out only the query errors. [#1338]
 - CDS: Apply MOCPy v0.5.* API changes. [#1343]
+- SDSS: Update to SDSS-IV URLs and general clean-up. [#1308]
 
 0.3.9 (2018-12-06)
 ------------------

astroquery/sdss/__init__.py

@@ -14,17 +14,14 @@ class Conf(_config.ConfigNamespace):
         'http://skyserver.sdss.org',
         'Base URL for catalog-related queries like SQL and Cross-ID.')
     sas_baseurl = _config.ConfigItem(
-        'http://data.sdss3.org/sas',
+        'https://data.sdss.org/sas',
         'Base URL for downloading data products like spectra and images.')
     timeout = _config.ConfigItem(
         60,
         'Time limit for connecting to SDSS server.')
+    default_release = _config.ConfigItem(14, 'Default SDSS data release.')
 
 
 conf = Conf()
 
 from .core import SDSS, SDSSClass
-
-import warnings
-warnings.warn("Experimental: SDSS has not yet been refactored to have its API "
-              "match the rest of astroquery (but it's nearly there).")

astroquery/sdss/core.py

@@ -34,7 +34,7 @@ class SDSSClass(BaseQuery):
     QUERY_URL_SUFFIX_DR_NEW = '/dr{dr}/en/tools/search/x_results.aspx'
     XID_URL_SUFFIX_OLD = '/dr{dr}/en/tools/crossid/x_crossid.aspx'
     XID_URL_SUFFIX_NEW = '/dr{dr}/en/tools/search/X_Results.aspx'
-    IMAGING_URL_SUFFIX = ('{base}/dr{dr}/boss/photoObj/frames/'
+    IMAGING_URL_SUFFIX = ('{base}/dr{dr}/{instrument}/photoObj/frames/'
                           '{rerun}/{run}/{camcol}/'
                           'frame-{band}-{run:06d}-{camcol}-'
                           '{field:04d}.fits.bz2')
@@ -59,7 +59,8 @@ class SDSSClass(BaseQuery):
     def query_crossid_async(self, coordinates, obj_names=None,
                             photoobj_fields=None, specobj_fields=None,
                             get_query_payload=False, timeout=TIMEOUT,
-                            radius=5. * u.arcsec, data_release=12, cache=True):
+                            radius=5. * u.arcsec,
+                            data_release=conf.default_release, cache=True):
         """
         Query using the cross-identification web interface.
 
@@ -172,7 +173,8 @@ def query_region_async(self, coordinates, radius=2. * u.arcsec,
                            fields=None, spectro=False, timeout=TIMEOUT,
                            get_query_payload=False, photoobj_fields=None,
                            specobj_fields=None, field_help=False,
-                           obj_names=None, data_release=12, cache=True):
+                           obj_names=None, data_release=conf.default_release,
+                           cache=True):
         """
         Used to query a region around given coordinates. Equivalent to
         the object cross-ID from the web interface.
@@ -263,7 +265,7 @@ def query_region_async(self, coordinates, radius=2. * u.arcsec,
     def query_specobj_async(self, plate=None, mjd=None, fiberID=None,
                             fields=None, timeout=TIMEOUT,
                             get_query_payload=False, field_help=False,
-                            data_release=12, cache=True):
+                            data_release=conf.default_release, cache=True):
         """
         Used to query the SpecObjAll table with plate, mjd and fiberID values.
 
@@ -337,7 +339,7 @@ def query_specobj_async(self, plate=None, mjd=None, fiberID=None,
     def query_photoobj_async(self, run=None, rerun=301, camcol=None,
                              field=None, fields=None, timeout=TIMEOUT,
                              get_query_payload=False, field_help=False,
-                             data_release=12, cache=True):
+                             data_release=conf.default_release, cache=True):
         """
         Used to query the PhotoObjAll table with run, rerun, camcol and field
         values.
@@ -418,7 +420,8 @@ def __sanitize_query(self, stmt):
             fsql += ' ' + line.split('--')[0]
         return fsql
 
-    def query_sql_async(self, sql_query, timeout=TIMEOUT, data_release=12,
+    def query_sql_async(self, sql_query, timeout=TIMEOUT,
+                        data_release=conf.default_release,
                         cache=True, **kwargs):
         """
         Query the SDSS database.
@@ -477,7 +480,8 @@ class = 'galaxy' \
     def get_spectra_async(self, coordinates=None, radius=2. * u.arcsec,
                           matches=None, plate=None, fiberID=None, mjd=None,
                           timeout=TIMEOUT, get_query_payload=False,
-                          data_release=12, cache=True, show_progress=True):
+                          data_release=conf.default_release, cache=True,
+                          show_progress=True):
         """
         Download spectrum from SDSS.
 
@@ -597,7 +601,8 @@ def get_spectra_async(self, coordinates=None, radius=2. * u.arcsec,
     @prepend_docstr_nosections(get_spectra_async.__doc__)
     def get_spectra(self, coordinates=None, radius=2. * u.arcsec,
                     matches=None, plate=None, fiberID=None, mjd=None,
-                    timeout=TIMEOUT, cache=True, data_release=12,
+                    timeout=TIMEOUT, cache=True,
+                    data_release=conf.default_release,
                     show_progress=True):
         """
         Returns
@@ -622,7 +627,8 @@ def get_spectra(self, coordinates=None, radius=2. * u.arcsec,
     def get_images_async(self, coordinates=None, radius=2. * u.arcsec,
                          matches=None, run=None, rerun=301, camcol=None,
                          field=None, band='g', timeout=TIMEOUT,
-                         get_query_payload=False, cache=True, data_release=12,
+                         get_query_payload=False, cache=True,
+                         data_release=conf.default_release,
                          show_progress=True):
         """
         Download an image from SDSS.
@@ -728,8 +734,11 @@ def get_images_async(self, coordinates=None, radius=2. * u.arcsec,
             for b in band:
                 # Download and read in image data
                 linkstr = self.IMAGING_URL_SUFFIX
+                instrument = 'boss'
+                if data_release > 12:
+                    instrument = 'eboss'
                 link = linkstr.format(base=conf.sas_baseurl, run=row['run'],
-                                      dr=data_release,
+                                      dr=data_release, instrument=instrument,
                                       rerun=row['rerun'], camcol=row['camcol'],
                                       field=row['field'], band=b)
 
@@ -743,7 +752,7 @@ def get_images_async(self, coordinates=None, radius=2. * u.arcsec,
     def get_images(self, coordinates=None, radius=2. * u.arcsec,
                    matches=None, run=None, rerun=301, camcol=None, field=None,
                    band='g', timeout=TIMEOUT, cache=True,
-                   get_query_payload=False, data_release=12,
+                   get_query_payload=False, data_release=conf.default_release,
                    show_progress=True):
         """
         Returns
@@ -865,7 +874,8 @@ def _args_to_payload(self, coordinates=None, radius=2. * u.arcsec,
                          plate=None, mjd=None, fiberID=None, run=None,
                          rerun=301, camcol=None, field=None,
                          photoobj_fields=None, specobj_fields=None,
-                         field_help=None, obj_names=None, data_release=12):
+                         field_help=None, obj_names=None,
+                         data_release=conf.default_release):
         """
         Construct the SQL query from the arguments.
 

astroquery/sdss/field_names.py

@@ -66,21 +66,21 @@ def _columns_json_to_table(jsonobj):
 # below here are builtin data files
 def _load_builtin_table_fields():
     key1 = ('PhotoObjAll',
-            'http://skyserver.sdss.org/dr12/en/tools/search/x_sql.aspx')
+            conf.skyserver_baseurl + '/dr12/en/tools/search/x_sql.aspx')
     _cached_table_fields[key1] = _columns_json_to_table(
         json.loads(get_pkg_data_contents('data/PhotoObjAll_dr12.json')))
     # PhotoObj and PhotoObjAll are the same in DR12
     key2 = ('PhotoObj',
-            'http://skyserver.sdss.org/dr12/en/tools/search/x_sql.aspx')
+            conf.skyserver_baseurl + '/dr12/en/tools/search/x_sql.aspx')
     _cached_table_fields[key2] = _cached_table_fields[key1]
 
     key1 = ('SpecObjAll',
-            'http://skyserver.sdss.org/dr12/en/tools/search/x_sql.aspx')
+            conf.skyserver_baseurl + '/dr12/en/tools/search/x_sql.aspx')
     _cached_table_fields[key1] = _columns_json_to_table(
         json.loads(get_pkg_data_contents('data/SpecObjAll_dr12.json')))
     # SpecObj and SpecObjAll are the same in DR12
     key2 = ('SpecObj',
-            'http://skyserver.sdss.org/dr12/en/tools/search/x_sql.aspx')
+            conf.skyserver_baseurl + '/dr12/en/tools/search/x_sql.aspx')
     _cached_table_fields[key2] = _cached_table_fields[key1]
 
 

astroquery/sdss/tests/test_sdss.py

@@ -124,6 +124,9 @@ def data_path(filename):
 # Test Case: Column of coordinates
 coords_column = Column(coords_list, name='coordinates')
 
+# List of all data releases.
+dr_list = list(range(1, sdss.conf.default_release + 1))
+
 
 # We are not testing queries for DR11 because it is not easily available to
 # query: "DR11 data are distributed primarily to provide reproducibility of
@@ -134,6 +137,8 @@ def url_tester(data_release):
         baseurl = 'http://skyserver.sdss.org/dr{}/en/tools/search/x_sql.asp'
     if data_release == 10:
         baseurl = 'http://skyserver.sdss.org/dr{}/en/tools/search/x_sql.aspx'
+    if data_release == 11:
+        return
     if data_release >= 12:
         baseurl = 'http://skyserver.sdss.org/dr{}/en/tools/search/x_results.aspx'
     assert sdss.SDSS._last_url == baseurl.format(data_release)
@@ -142,21 +147,19 @@ def url_tester(data_release):
 def url_tester_crossid(data_release):
     if data_release < 11:
         baseurl = 'http://skyserver.sdss.org/dr{}/en/tools/crossid/x_crossid.aspx'
+    if data_release == 11:
+        return
     if data_release >= 12:
         baseurl = 'http://skyserver.sdss.org/dr{}/en/tools/search/X_Results.aspx'
     assert sdss.SDSS._last_url == baseurl.format(data_release)
 
 
 def compare_xid_data(xid, data):
-    if six.PY3:
-        pytest.xfail('xid/data comparison fails in PY3 because the instrument '
-                     'column is bytes in xid and str in data')
-    else:
-        for col in xid.colnames:
-            if xid[col].dtype.type is np.string_:
-                assert xid[col] == data[col]
-            else:
-                assert_allclose(xid[col], data[col])
+    for col in xid.colnames:
+        if xid[col].dtype.type is np.string_:
+            assert xid[col] == data[col]
+        else:
+            assert_allclose(xid[col], data[col])
 
 
 def image_tester(images, filetype):
@@ -167,7 +170,7 @@ def image_tester(images, filetype):
     assert images[0][0].data == data[0].data
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_spectrum(patch_get, patch_get_readable_fileobj, dr,
                        coords=coords):
     xid = sdss.SDSS.query_region(coords, data_release=dr, spectro=True)
@@ -177,20 +180,20 @@ def test_sdss_spectrum(patch_get, patch_get_readable_fileobj, dr,
     url_tester(dr)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_spectrum_mjd(patch_get, patch_get_readable_fileobj, dr):
     sp = sdss.SDSS.get_spectra(plate=2345, fiberID=572, data_release=dr)
     image_tester(sp, 'spectra')
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_spectrum_coords(patch_get, patch_get_readable_fileobj, dr,
                               coords=coords):
     sp = sdss.SDSS.get_spectra(coords, data_release=dr)
     image_tester(sp, 'spectra')
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_sql(patch_get, patch_get_readable_fileobj, dr):
     query = """
             select top 10
@@ -213,7 +216,7 @@ class = 'galaxy'
     url_tester(dr)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_image_from_query_region(patch_get, patch_get_readable_fileobj,
                                       dr, coords=coords):
     xid = sdss.SDSS.query_region(coords, data_release=dr)
@@ -223,13 +226,13 @@ def test_sdss_image_from_query_region(patch_get, patch_get_readable_fileobj,
     url_tester(dr)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_image_run(patch_get, patch_get_readable_fileobj, dr):
     img = sdss.SDSS.get_images(run=1904, camcol=3, field=164, data_release=dr)
     image_tester(img, 'images')
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_image_coord(patch_get, patch_get_readable_fileobj, dr,
                           coord=coords):
     img = sdss.SDSS.get_images(coords, data_release=dr)
@@ -241,7 +244,7 @@ def test_sdss_template(patch_get, patch_get_readable_fileobj):
     image_tester(template, 'spectra')
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_specobj(patch_get, dr):
     xid = sdss.SDSS.query_specobj(plate=2340, data_release=dr)
     data = Table.read(data_path(DATA_FILES['spectra_id']),
@@ -255,7 +258,7 @@ def test_sdss_specobj(patch_get, dr):
     url_tester(dr)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_sdss_photoobj(patch_get, dr):
     xid = sdss.SDSS.query_photoobj(
         run=1904, camcol=3, field=164, data_release=dr)
@@ -269,7 +272,7 @@ def test_sdss_photoobj(patch_get, dr):
     url_tester(dr)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_list_coordinates(patch_get, dr):
     xid = sdss.SDSS.query_region(coords_list, data_release=dr)
     data = Table.read(data_path(DATA_FILES['images_id']),
@@ -281,7 +284,7 @@ def test_list_coordinates(patch_get, dr):
     compare_xid_data(xid, data)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_column_coordinates(patch_get, dr):
     xid = sdss.SDSS.query_region(coords_column, data_release=dr)
     data = Table.read(data_path(DATA_FILES['images_id']),
@@ -309,7 +312,7 @@ def test_images_timeout(patch_get, patch_get_readable_fileobj_slow):
         sdss.SDSS.get_images(run=1904, camcol=3, field=164)
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_query_crossid(patch_post, dr):
     xid = sdss.SDSS.query_crossid(coords_column, data_release=dr)
     data = Table.read(data_path(DATA_FILES['images_id']),
@@ -325,7 +328,7 @@ def test_query_crossid(patch_post, dr):
 # ===========
 # Payload tests
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_list_coordinates_payload(patch_get, dr):
     expect = ("SELECT DISTINCT "
               "p.ra, p.dec, p.objid, p.run, p.rerun, p.camcol, p.field "
@@ -341,7 +344,7 @@ def test_list_coordinates_payload(patch_get, dr):
     assert query_payload['format'] == 'csv'
 
 
-@pytest.mark.parametrize("dr", [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 12])
+@pytest.mark.parametrize("dr", dr_list)
 def test_column_coordinates_payload(patch_get, dr):
     expect = ("SELECT DISTINCT "
               "p.ra, p.dec, p.objid, p.run, p.rerun, p.camcol, p.field "

astroquery/sdss/tests/test_sdss_remote.py

@@ -1,5 +1,6 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 import numpy as np
+from numpy.testing import assert_allclose
 import pytest
 
 from astropy import coordinates
@@ -99,7 +100,13 @@ def test_sdss_specobj(self):
         xid = sdss.SDSS.query_specobj(plate=2340)
         assert isinstance(xid, Table)
         for row in table:
-            assert row in xid
+            i = np.nonzero(xid['specobjid'] == row['specobjid'])[0]
+            assert len(i) == 1
+            for j, c in enumerate(colnames):
+                if dtypes[j] is float:
+                    assert_allclose(xid[i][c], row[c])
+                else:
+                    assert xid[i][c] == row[c]
 
     def test_sdss_photoobj(self):
         colnames = ['ra', 'dec', 'objid', 'run', 'rerun', 'camcol', 'field']
@@ -120,7 +127,13 @@ def test_sdss_photoobj(self):
         xid = sdss.SDSS.query_photoobj(run=1904, camcol=3, field=164)
         assert isinstance(xid, Table)
         for row in table:
-            assert row in xid
+            i = np.nonzero(xid['objid'] == row['objid'])[0]
+            assert len(i) == 1
+            for j, c in enumerate(colnames):
+                if dtypes[j] is float:
+                    assert_allclose(xid[i][c], row[c])
+                else:
+                    assert xid[i][c] == row[c]
 
     @pytest.mark.xfail(reason=("Timeout isn't raised since switching to "
                                "self._request, fix it before merging #586"))