Merge pull request #3194 from cds-astro/fix-xmatch-error-message-on-w…

…rong-catalog-name

Make the API more flexible (allow to ommit `vizier:` when crossmatching with a vizier catalog)

CHANGES.rst

@@ -13,6 +13,12 @@ ipac.nexsci.nasa_exoplanet_archive
 
 - Fixed InvalidTableError for DI_STARS_EXEP and TD tables. [#3189]
 
+xmatch
+^^^^^^
+
+- the API is more flexible: you can now ommit the ``vizier:`` before the catalog name
+  when crossmatching with a vizier table [#3194]
+
 
 Infrastructure, Utility and Other Changes and Additions
 -------------------------------------------------------

astroquery/xmatch/core.py

@@ -94,7 +94,7 @@ def query_async(self, cat1, cat2, max_distance, *, colRA1=None, colDec1=None,
             The HTTP response returned from the service.
         """
         if max_distance > 180 * u.arcsec:
-            raise ValueError('max_distance argument must not be greater than 180')
+            raise ValueError('max_distance argument must not be greater than 180".')
         payload = {'request': 'xmatch',
                    'distMaxArcsec': max_distance.to(u.arcsec).value,
                    'RESPONSEFORMAT': 'votable',
@@ -126,6 +126,10 @@ def _prepare_sending_table(self, cat_index, payload, kwargs, cat, colRA, colDec)
         '''
         catstr = 'cat{0}'.format(cat_index)
         if isinstance(cat, str):
+            if (self.is_table_available(cat) and not cat.startswith("vizier:")):
+                # if we detect that the given name is a vizier table, we can make
+                # it comply to the API, see issue #3191
+                cat = f"vizier:{cat}"
             payload[catstr] = cat
         else:
             # create the dictionary of uploaded files
@@ -181,7 +185,7 @@ def is_table_available(self, table_id):
         if not isinstance(table_id, str):
             return False
 
-        if (table_id[:7] == 'vizier:'):
+        if table_id.startswith('vizier:'):
             table_id = table_id[7:]
 
         return table_id in self.get_available_tables()

astroquery/xmatch/tests/test_xmatch.py

@@ -46,10 +46,9 @@ def request_mockreturn(method, url, data, **kwargs):
 
 
 def test_xmatch_query_invalid_max_distance():
-    with pytest.raises(ValueError) as ex:
+    with pytest.raises(ValueError,
+                       match='max_distance argument must not be greater than 180"'):
         XMatch().query_async('', '', 181 * arcsec)
-        assert str(ex.value) == (
-            'max_distance argument must not be greater than 180')
 
 
 def test_get_available_tables(monkeypatch):
@@ -125,3 +124,35 @@ def test_table_not_available(monkeypatch):
     # reproduces #1464
     with pytest.raises(ValueError, match=f"'{re.escape(cat1)}' is not available *"):
         xm.query_async(cat1=cat1, cat2=cat2, max_distance=5 * arcsec)
+
+
+def test_prepare_sending_tables(monkeypatch):
+    xm = XMatch()
+    monkeypatch.setattr(xm, '_request', request_mockreturn)
+
+    # if it's a valid vizier table, prepend vizier:
+    payload = {}
+    xm._prepare_sending_table(1, payload, {}, "II/316/gps6", None, None)
+    assert payload == {'cat1': 'vizier:II/316/gps6'}
+    # also works if vizier: is already given by the user
+    payload = {}
+    xm._prepare_sending_table(1, payload, {}, "vizier:II/316/gps6", None, None)
+    assert payload == {'cat1': 'vizier:II/316/gps6'}
+
+    # otherwise colRa1 and colDec1 have to be given
+    with pytest.raises(ValueError, match="'test' is not available on the XMatch server."):
+        xm._prepare_sending_table(1, payload, {}, "test", None, None)
+    payload = {}
+    # this mimics the url case
+    xm._prepare_sending_table(1, payload, {}, "test", "ra", "dec")
+    assert payload == {'cat1': 'test', 'colRA1': 'ra', 'colDec1': 'dec'}
+
+    # if cat is not a string, then the payload has to include the file
+    payload = {}
+    kwargs = {}
+    cat = Table({'a': [0, 1, 2], 'b': [3, 4, 5]})
+    xm._prepare_sending_table(1, payload, kwargs, cat, "a", "b")
+    assert payload == {'colRA1': 'a', 'colDec1': 'b'}
+    assert (kwargs == {'files': {'cat1': ('cat1.csv', 'a,b\n0,3\n1,4\n2,5\n')}}
+            # for windows systems
+            or kwargs == {'files': {'cat1': ('cat1.csv', 'a,b\r\n0,3\r\n1,4\r\n2,5\r\n')}})

astroquery/xmatch/tests/test_xmatch_remote.py

@@ -112,6 +112,12 @@ def test_xmatch_query_astropy_table(self, xmatch, remote_table):
             else:
                 assert_allclose(table[col], remote_table[col])
 
+    def test_two_remote_tables(self, xmatch):
+        # we also test that omitting vizier: works
+        result = xmatch.query("simbad", "J/A+A/113/61/table2", max_distance=0.5*arcsec)
+        assert {"main_id", "MH"}.issubset(result.colnames)
+        assert all(result["angDist"] < 0.5)
+
     @pytest.mark.skipif('regions' not in sys.modules,
                         reason="requires astropy-regions")
     def test_xmatch_query_with_cone_area(self, xmatch):