address review comments

Author: zoghbi-a

astroquery/heasarc/core.py

@@ -99,7 +99,7 @@ def _get_default_columns(self, catalog_name):
         ]
         meta.sort('value')
         defaults = meta['par']
-        return defaults
+        return list(defaults)
 
     def get_default_radius(self, catalog_name):
         """Get a mission-appropriate default radius for a catalog
@@ -118,10 +118,10 @@ def get_default_radius(self, catalog_name):
             (self._meta['table'] == catalog_name)
             & (self._meta['par'] == '')
         ]
-        radius = np.double(meta['value'][0]) * u.arcmin
+        radius = np.float32(meta['value'][0]) * u.arcmin
         return radius
 
-    def set_session(self, session):
+    def _set_session(self, session):
         """Set requests.Session to use when querying the data
 
         Parameters
@@ -424,8 +424,8 @@ def query_region(self, position=None, catalog=None, radius=None, *,
         response = self.query_tap(query=adql, maxrec=maxrec)
 
         # save the response in case we want to use it later
-        self._query_result = response
-        self._catalog_name = catalog
+        self._last_result = response
+        self._last_catalog_name = catalog
 
         table = response.to_table()
         if add_offset:
@@ -484,13 +484,13 @@ def locate_data(self, query_result=None, catalog_name=None):
         """
         if query_result is None:
             if (
-                not hasattr(self, '_query_result')
-                or self._query_result is None
+                not hasattr(self, '_last_result')
+                or self._last_result is None
             ):
                 raise ValueError('query_result is None, and none '
                                  'found from a previous search')
             else:
-                query_result = self._query_result
+                query_result = self._last_result
 
         if not isinstance(query_result, Table):
             raise TypeError('query_result need to be an astropy.table.Table')
@@ -502,7 +502,7 @@ def locate_data(self, query_result=None, catalog_name=None):
                              'query_region or a subset.')
 
         if catalog_name is None:
-            catalog_name = self._catalog_name
+            catalog_name = self._last_catalog_name
         if not (
             isinstance(catalog_name, str)
             and catalog_name in self.tap.tables.keys()
@@ -661,10 +661,10 @@ def _download_heasarc(self, links, location='.'):
         }
 
         # get local_filepath name
-        local_filepath = f'{location}/xamin.tar'
+        local_filepath = f'{location}/heasarc-data.tar'
         iname = 1
         while os.path.exists(local_filepath):
-            local_filepath = f'{location}/xamin.{iname}.tar'
+            local_filepath = f'{location}/heasarc-data.{iname}.tar'
             iname += 1
 
         log.info(f'Downloading to {local_filepath} ...')

astroquery/heasarc/tests/test_heasarc.py

@@ -8,7 +8,7 @@
 from astropy.table import Table
 import astropy.units as u
 
-from astroquery.heasarc import Heasarc
+from astroquery.heasarc import Heasarc, HeasarcClass
 from astroquery.exceptions import InvalidQueryError
 
 try:
@@ -199,6 +199,8 @@ def test_no_catalog():
 
 
 def test_tap_def():
+    # Use a new HeasarcClass object
+    Heasarc = HeasarcClass()
     assert Heasarc._tap is None
     Heasarc.tap
     assert Heasarc._tap is not None
@@ -208,6 +210,8 @@ def test_meta_def():
     class MockResult:
         def to_table(self):
             return Table({'value': ['1.5', '1.2', '-0.3']})
+    # Use a new HeasarcClass object
+    Heasarc = HeasarcClass()
     assert Heasarc._meta_info is None
     with patch('astroquery.heasarc.core.HeasarcClass.query_tap') as mock_query_tap:
         mock_query_tap.return_value = MockResult()
@@ -216,18 +220,22 @@ def to_table(self):
 
 
 def test__get_default_columns(mock_meta):
+    # Use a new HeasarcClass object
+    Heasarc = HeasarcClass()
     cols = Heasarc._get_default_columns('tab1')
     assert list(cols) == ['p1', 'p3']
 
 
 def test__get_default_radius(mock_meta):
+    # Use a new HeasarcClass object
+    Heasarc = HeasarcClass()
     radius = Heasarc.get_default_radius('tab1')
     assert radius.value == 3.0
 
 
-def test_set_session():
+def test__set_session():
     with pytest.raises(ValueError):
-        Heasarc.set_session('not session object')
+        Heasarc._set_session('not session object')
 
 
 def test__list_catalogs(mock_tap):

astroquery/heasarc/tests/test_heasarc_remote.py

@@ -1,8 +1,8 @@
 # Licensed under a 3-clause BSD style license - see LICENSE.rst
 
 import os
-import shutil
 import pytest
+import tempfile
 import astropy.units as u
 from astropy.table import Table
 from astropy.coordinates import SkyCoord
@@ -83,6 +83,7 @@ def test_query_region_cone(self, coordinates):
             spatial="cone",
             columns="*",
             radius=1 * u.arcmin,
+            add_offset=True
         )
         assert isinstance(result, Table)
         assert len(result) == 3
@@ -95,11 +96,12 @@ def test_query_columns_radius(self):
         """
         result = Heasarc.query_region(
             "NGC 4151", catalog="suzamaster", columns="ra,dec,obsid",
-            radius=10 * u.arcmin
+            radius=10 * u.arcmin,
+            add_offset=True
         )
         assert len(result) == 4
         # assert only selected columns are returned
-        assert result.colnames == ["ra", "dec", "obsid", "search_offset_"]
+        assert result.colnames == ["ra", "dec", "obsid", "search_offset"]
 
     def test_query_region_box(self):
         result = Heasarc.query_region(
@@ -196,9 +198,9 @@ def test_download_data__heasarc_file(self):
                  f"data/archive/{filename}")
             ]
         })
-        Heasarc.download_data(tab, host="heasarc", location=".")
-        assert os.path.exists(filename)
-        os.remove(filename)
+        with tempfile.TemporaryDirectory() as tmpdir:
+            Heasarc.download_data(tab, host="heasarc", location=tmpdir)
+            assert os.path.exists(f'{tmpdir}/{filename}')
 
     def test_download_data__heasarc_folder(self):
         tab = Table({
@@ -207,21 +209,22 @@ def test_download_data__heasarc_folder(self):
                  "AO10/P91129/91129-01-68-00A/stdprod")
             ]
         })
-        Heasarc.download_data(tab, host="heasarc", location=".")
-        assert os.path.exists("stdprod")
-        assert os.path.exists("stdprod/FHed_1791a7b9-1791a931.gz")
-        assert os.path.exists("stdprod/FHee_1791a7b9-1791a92f.gz")
-        assert os.path.exists("stdprod/FHef_1791a7b9-1791a92f.gz")
-        shutil.rmtree("stdprod")
+        with tempfile.TemporaryDirectory() as tmpdir:
+            Heasarc.download_data(tab, host="heasarc", location=tmpdir)
+            assert os.path.exists(f"{tmpdir}/stdprod")
+            assert os.path.exists(f"{tmpdir}/stdprod/FHed_1791a7b9-1791a931.gz")
+            assert os.path.exists(f"{tmpdir}/stdprod/FHee_1791a7b9-1791a92f.gz")
+            assert os.path.exists(f"{tmpdir}/stdprod/FHef_1791a7b9-1791a92f.gz")
 
     def test_download_data__s3_file(self):
         filename = "00README"
         tab = Table(
             {"aws": [f"s3://nasa-heasarc/rxte/data/archive/{filename}"]}
         )
-        Heasarc.download_data(tab, host="aws", location=".")
-        assert os.path.exists(filename)
-        os.remove(filename)
+        with tempfile.TemporaryDirectory() as tmpdir:
+            Heasarc.enable_cloud(provider='aws', profile=None)
+            Heasarc.download_data(tab, host="aws", location=tmpdir)
+            assert os.path.exists(f'{tmpdir}/{filename}')
 
     @pytest.mark.parametrize("slash", ["", "/"])
     def test_download_data__s3_folder(self, slash):
@@ -233,12 +236,13 @@ def test_download_data__s3_folder(self, slash):
                 ]
             }
         )
-        Heasarc.download_data(tab, host="aws", location=".")
-        assert os.path.exists("stdprod")
-        assert os.path.exists("stdprod/FHed_1791a7b9-1791a931.gz")
-        assert os.path.exists("stdprod/FHee_1791a7b9-1791a92f.gz")
-        assert os.path.exists("stdprod/FHef_1791a7b9-1791a92f.gz")
-        shutil.rmtree("stdprod")
+        with tempfile.TemporaryDirectory() as tmpdir:
+            Heasarc.enable_cloud(provider='aws', profile=None)
+            Heasarc.download_data(tab, host="aws", location=tmpdir)
+            assert os.path.exists(f"{tmpdir}/stdprod")
+            assert os.path.exists(f"{tmpdir}/stdprod/FHed_1791a7b9-1791a931.gz")
+            assert os.path.exists(f"{tmpdir}/stdprod/FHee_1791a7b9-1791a92f.gz")
+            assert os.path.exists(f"{tmpdir}/stdprod/FHef_1791a7b9-1791a92f.gz")
 
     def test_query_mission_columns(self):
         with pytest.warns(AstropyDeprecationWarning):