Merge pull request #2475 from at88mph/err-unused-args

BUG: raise for unsupported arguments for alma

Author: bsipocz

CHANGES.rst

@@ -24,6 +24,7 @@ alma
 ^^^^
 
 - Fixed a regression to handle arrays of string input for the ``query`` methods. [#2094]
+- Throws an error when an unsupported ``kwargs`` (or argument) is passed in to a function. [#2475]
 
 
 astrometry.net

astroquery/alma/core.py

@@ -162,6 +162,7 @@
 def _gen_sql(payload):
     sql = 'select * from ivoa.obscore'
     where = ''
+    unused_payload = payload.copy()
     if payload:
         for constraint in payload:
             for attrib_category in ALMA_FORM_KEYS.values():
@@ -181,6 +182,18 @@ def _gen_sql(payload):
                             else:
                                 where = ' WHERE '
                             where += attrib_where
+
+                        # Delete this key to see what's left over afterward
+                        #
+                        # Use pop to avoid the slight possibility of trying to remove
+                        # an already removed key
+                        unused_payload.pop(constraint)
+
+    if unused_payload:
+        # Left over (unused) constraints passed.  Let the user know.
+        remaining = [f'{p} -> {unused_payload[p]}' for p in unused_payload]
+        raise TypeError(f'Unsupported arguments were passed:\n{remaining}')
+
     return sql + where
 
 
@@ -296,7 +309,8 @@ def query_region_async(self, coordinate, radius, *, public=True,
                                 payload=payload, **kwargs)
 
     def query_async(self, payload, *, public=True, science=True,
-                    legacy_columns=False, get_query_payload=None, **kwargs):
+                    legacy_columns=False, get_query_payload=None,
+                    maxrec=None, **kwargs):
         """
         Perform a generic query with user-specified payload
 
@@ -313,6 +327,10 @@ def query_async(self, payload, *, public=True, science=True,
         legacy_columns : bool
             True to return the columns from the obsolete ALMA advanced query,
             otherwise return the current columns based on ObsCore model.
+        get_query_payload : bool
+            Flag to indicate whether to simply return the payload.
+        maxrec : integer
+            Cap on the amount of records returned.  Default is no limit.
 
         Returns
         -------
@@ -340,7 +358,7 @@ def query_async(self, payload, *, public=True, science=True,
             return payload
 
         query = _gen_sql(payload)
-        result = self.query_tap(query, maxrec=payload.get('maxrec', None))
+        result = self.query_tap(query, maxrec=maxrec)
         if result is not None:
             result = result.to_table()
         else:
@@ -588,7 +606,7 @@ def is_proprietary(self, uid):
         proprietary or not.
         """
         query = "select distinct data_rights from ivoa.obscore where " \
-                "obs_id='{}'".format(uid)
+                "member_ous_uid='{}'".format(uid)
         result = self.query_tap(query)
         if result:
             tableresult = result.to_table()

astroquery/alma/tests/test_alma.py

@@ -243,6 +243,18 @@ def test_pol_sql():
         common_select + " WHERE (pol_states='/XX/' OR pol_states='/XX/YY/')"
 
 
+def test_unused_args():
+    alma = Alma()
+    alma._get_dataarchive_url = Mock()
+    # with patch('astroquery.alma.tapsql.coord.SkyCoord.from_name') as name_mock, pytest.raises(TypeError) as typeError:
+    with patch('astroquery.alma.tapsql.coord.SkyCoord.from_name') as name_mock:
+        with pytest.raises(TypeError) as typeError:
+            name_mock.return_value = SkyCoord(1, 2, unit='deg')
+            alma.query_object('M13', public=False, bogus=True, nope=False, band_list=[3])
+
+        assert "['bogus -> True', 'nope -> False']" in str(typeError.value)
+
+
 def test_query():
     # Tests the query and return values
     tap_mock = Mock()

astroquery/alma/tests/test_alma_remote.py

@@ -87,7 +87,8 @@ def test_freq(self, alma):
 
     def test_bands(self, alma):
         payload = {'band_list': ['5', '7']}
-        result = alma.query(payload)
+        # Added maxrec here as downloading and reading the results take too long.
+        result = alma.query(payload, maxrec=1000)
         assert len(result) > 0
         for row in result:
             assert ('5' in row['band_list']) or ('7' in row['band_list'])
@@ -136,7 +137,7 @@ def test_data_proprietary(self, alma):
         assert not alma.is_proprietary('uid://A001/X12a3/Xe9')
         IVOA_DATE_FORMAT = "%Y-%m-%dT%H:%M:%S.%f"
         now = datetime.utcnow().strftime(IVOA_DATE_FORMAT)[:-3]
-        query = "select top 1 obs_id from ivoa.obscore where " \
+        query = "select top 1 member_ous_uid from ivoa.obscore where " \
                 "obs_release_date > '{}'".format(now)
         result = alma.query_tap(query)
         assert len(result.table) == 1
@@ -146,6 +147,7 @@ def test_data_proprietary(self, alma):
         with pytest.raises(AttributeError):
             alma.is_proprietary('uid://NON/EXI/STING')
 
+    @pytest.mark.xfail(reason="Depends on PR 2438 (https://github.com/astropy/astroquery/pull/2438)")
     def test_data_info(self, temp_dir, alma):
         alma.cache_location = temp_dir
 
@@ -257,6 +259,7 @@ def test_doc_example(self, temp_dir, alma):
         gc_data = alma.query_region(galactic_center, 1 * u.deg)
         # assert len(gc_data) >= 425 # Feb 8, 2016
         assert len(gc_data) >= 50  # Nov 16, 2016
+        content_length_column_name = 'content_length'
 
         uids = np.unique(m83_data['Member ous id'])
         if ASTROPY_LT_4_1:
@@ -271,11 +274,11 @@ def test_doc_example(self, temp_dir, alma):
         assert X30.sum() == 4  # Jul 13, 2020
         assert X31.sum() == 4  # Jul 13, 2020
         mous1 = alma.get_data_info('uid://A001/X11f/X30')
-        totalsize_mous1 = mous1['size'].sum() * u.Unit(mous1['size'].unit)
+        totalsize_mous1 = mous1[content_length_column_name].sum() * u.Unit(mous1[content_length_column_name].unit)
         assert (totalsize_mous1.to(u.B) > 1.9*u.GB)
 
         mous = alma.get_data_info('uid://A002/X3216af/X31')
-        totalsize_mous = mous['size'].sum() * u.Unit(mous['size'].unit)
+        totalsize_mous = mous[content_length_column_name].sum() * u.Unit(mous[content_length_column_name].unit)
         # More recent ALMA request responses do not include any information
         # about file size, so we have to allow for the possibility that all
         # file sizes are replaced with -1
@@ -313,11 +316,13 @@ def test_misc(self, alma):
         result = alma.query(payload={'pi_name': '*Bally*'}, public=False,
                             maxrec=10)
         assert result
-        result.write('/tmp/alma-onerow.txt', format='ascii')
+        # Add overwrite=True in case the test previously died unexpectedly
+        # and left the temp file.
+        result.write('/tmp/alma-onerow.txt', format='ascii', overwrite=True)
         for row in result:
             assert 'Bally' in row['obs_creator_name']
         result = alma.query(payload=dict(project_code='2016.1.00165.S'),
-                            public=False, cache=False)
+                            public=False)
         assert result
         for row in result:
             assert '2016.1.00165.S' == row['proposal_id']
@@ -336,8 +341,7 @@ def test_misc(self, alma):
 
         result = alma.query_region(
             coordinates.SkyCoord('5:35:14.461 -5:21:54.41', frame='fk5',
-                                 unit=(u.hour, u.deg)), radius=0.034 * u.deg,
-            payload={'energy.frequency-asu': '215 .. 220'})
+                                 unit=(u.hour, u.deg)), radius=0.034 * u.deg)
 
         result = alma.query(payload=dict(project_code='2012.*',
                                          public_data=True))