integrating better with astropy config framework

Author: ceb8

astroquery/__init__.py

@@ -54,18 +54,17 @@ def _get_bibtex():
 # Set up cache configuration
 class Conf(_config.ConfigNamespace):
 
-    default_cache_timeout = _config.ConfigItem(
-          604800,  # 1 week
-          'Astroquery-wide default cache timeout (seconds).'
-          )
-    default_cache_location = _config.ConfigItem(
-          os.path.join(_config.paths.get_cache_dir(), 'astroquery'),
-          'Astroquery default cache location (within astropy cache).'
-          )
-    default_cache_active = _config.ConfigItem(
+    cache_timeout = _config.ConfigItem(
+        604800,  # 1 week
+        'Astroquery-wide cache timeout (seconds).',
+        cfgtype='integer'
+    )
+
+    cache_active = _config.ConfigItem(
         True,
-        "Astroquery global cache usage, False turns off all caching."
-        )
+        "Astroquery global cache usage, False turns off all caching.",
+        cfgtype='boolean'
+    )
 
 
 conf = Conf()

astroquery/query.py

@@ -33,9 +33,6 @@ def to_cache(response, cache_file):
     if hasattr(response, 'request'):
         for key in tuple(response.request.hooks.keys()):
             del response.request.hooks[key]
-
-    chache_dir, _ = os.path.split(cache_file)
-    Path(chache_dir).mkdir(parents=True, exist_ok=True)
     with open(cache_file, "wb") as f:
         pickle.dump(response, f)
 
@@ -191,7 +188,7 @@ def __init__(self):
                     olduseragent=S.headers['User-Agent']))
 
         self.name = self.__class__.__name__.split("Class")[0]
-        self.reset_cache_preferences()
+        self.cache_location = None
 
     def __call__(self, *args, **kwargs):
         """ init a fresh copy of self """
@@ -231,23 +228,24 @@ def _response_hook(self, response, *args, **kwargs):
                     f"-----------------------------------------", '\t')
             log.log(5, f"HTTP response\n{response_log}")
 
-    def clear_cache(self):
-        """Removes all cache files."""
+    @property
+    def _cache_location(self):
+        cl = self.cache_location or os.path.join(paths.get_cache_dir(), 'astroquery', self.name)
+        Path(cl).mkdir(parents=True, exist_ok=True)
+        return cl
 
-        cache_files = [x for x in os.listdir(self.cache_location) if x.endswith("pickle")]
-        for fle in cache_files:
-            os.remove(os.path.join(self.cache_location, fle))
+    def get_cache_location(self):
+        return self._cache_location
 
-    def reset_cache_preferences(self):
-        """Resets cache preferences to default values"""
+    def reset_cache_location(self):
+        self.cache_location = None
 
-        self.cache_location = os.path.join(
-            conf.default_cache_location,
-            self.__class__.__name__.split("Class")[0])
-        os.makedirs(self.cache_location, exist_ok=True)
+    def clear_cache(self):
+        """Removes all cache files."""
 
-        self.cache_active = conf.default_cache_active
-        self.cache_timeout = conf.default_cache_timeout
+        cache_files = [x for x in os.listdir(self._cache_location) if x.endswith("pickle")]
+        for fle in cache_files:
+            os.remove(os.path.join(self._cache_location, fle))
 
     def _request(self, method, url,
                  params=None, data=None, headers=None,
@@ -319,7 +317,7 @@ def _request(self, method, url,
             json=json
         )
 
-        if (cache is not False) and self.cache_active:
+        if (cache is not False) and conf.cache_active:
             cache = True
         else:
             cache = False
@@ -331,7 +329,7 @@ def _request(self, method, url,
                 # ":" so replace them with an underscore
                 local_filename = local_filename.replace(':', '_')
 
-            local_filepath = os.path.join(savedir or self.cache_location or '.', local_filename)
+            local_filepath = os.path.join(savedir or self._cache_location or '.', local_filename)
 
             response = self._download_file(url, local_filepath, cache=cache,
                                            continuation=continuation, method=method,
@@ -343,23 +341,23 @@ def _request(self, method, url,
                 return local_filepath
         else:
             query = AstroQuery(method, url, **req_kwargs)
-            if ((self.cache_location is None) or (not self.cache_active) or (not cache)):
-                with suspend_cache(self):
+            if ((self._cache_location is None) or (not cache)):
+                with conf.set_temp("cache_active", False):
                     response = query.request(self._session, stream=stream,
                                              auth=auth, verify=verify,
                                              allow_redirects=allow_redirects,
                                              json=json)
             else:
-                response = query.from_cache(self.cache_location, self.cache_timeout)
+                response = query.from_cache(self._cache_location, conf.cache_timeout)
                 if not response:
                     response = query.request(self._session,
-                                             self.cache_location,
+                                             self._cache_location,
                                              stream=stream,
                                              auth=auth,
                                              allow_redirects=allow_redirects,
                                              verify=verify,
                                              json=json)
-                    to_cache(response, query.request_file(self.cache_location))
+                    to_cache(response, query.request_file(self._cache_location))
 
             self._last_query = query
             return response
@@ -486,23 +484,6 @@ def _download_file(self, url, local_filepath, timeout=None, auth=None,
         return response
 
 
-class suspend_cache:
-    """
-    A context manager that suspends caching.
-    """
-
-    def __init__(self, obj):
-        self.obj = obj
-        self.original_cache_setting = self.obj.cache_active
-
-    def __enter__(self):
-        self.obj.cache_active = False
-
-    def __exit__(self, exc_type, exc_value, traceback):
-        self.obj.cache_active = self.original_cache_setting
-        return False
-
-
 class QueryWithLogin(BaseQuery):
     """
     This is the base class for all the query classes which are required to
@@ -555,7 +536,7 @@ def _login(self, *args, **kwargs):
         pass
 
     def login(self, *args, **kwargs):
-        with suspend_cache(self):
+        with conf.set_temp("cache_active", False):
             self._authenticated = self._login(*args, **kwargs)
         return self._authenticated
 

astroquery/tests/test_cache.py

@@ -3,8 +3,12 @@
 import os
 
 from time import sleep
+from pathlib import Path
+
+from astropy.config import paths
 
 from astroquery.query import QueryWithLogin
+from astroquery import conf
 
 URL1 = "http://fakeurl.edu"
 URL2 = "http://fakeurl.ac.uk"
@@ -46,68 +50,99 @@ def _login(self, username):
             return False
 
 
-def test_cache_reset():
-    mytest = TestClass()
-    assert mytest.cache_active
+def test_conf():
+    default_timeout = conf.cache_timeout
+    default_active = conf.cache_active
+
+    assert default_timeout == 604800
+    assert default_active is True
 
-    default_timeout = mytest.cache_timeout
-    default_loc = mytest.cache_location
+    with conf.set_temp("cache_timeout", 5):
+        assert conf.cache_timeout == 5
 
-    mytest.cache_timeout = 5
-    mytest.cache_location = "new/location"
+    with conf.set_temp("cache_active", False):
+        assert conf.cache_active is False
 
-    mytest.reset_cache_preferences()
+    assert conf.cache_timeout == default_timeout
+    assert conf.cache_active == default_active
 
-    assert mytest.cache_timeout == default_timeout
-    assert mytest.cache_location == default_loc
+    conf.cache_timeout = 5
+    conf.cache_active = False
+    conf.reset()
+
+    assert conf.cache_timeout == default_timeout
+    assert conf.cache_active == default_active
 
 
 def test_basic_caching():
 
     mytest = TestClass()
-    assert mytest.cache_active
+    assert conf.cache_active
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0
 
     set_response(TEXT1)
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT1
-    assert len(os.listdir(mytest.cache_location)) == 1
+    assert len(os.listdir(mytest.get_cache_location())) == 1
 
     set_response(TEXT2)
 
     resp = mytest.test_func(URL2)  # query that has not been cached
     assert resp.content == TEXT2
-    assert len(os.listdir(mytest.cache_location)) == 2
+    assert len(os.listdir(mytest.get_cache_location())) == 2
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT1  # query that was cached
-    assert len(os.listdir(mytest.cache_location)) == 2  # no new cache file
+    assert len(os.listdir(mytest.get_cache_location())) == 2  # no new cache file
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT2  # Now get new response
 
 
+def test_change_location(tmpdir):
+
+    mytest = TestClass()
+    default_cache_location = mytest.get_cache_location()
+
+    assert paths.get_cache_dir() in default_cache_location
+    assert "astroquery" in mytest.get_cache_location()
+    assert mytest.name in mytest.get_cache_location()
+
+    new_loc = os.path.join(tmpdir, "new_dir")
+    mytest.cache_location = new_loc
+    assert mytest.get_cache_location() == new_loc
+
+    mytest.reset_cache_location()
+    assert mytest.get_cache_location() == default_cache_location
+
+    Path(new_loc).mkdir(parents=True, exist_ok=True)
+    with paths.set_temp_cache(new_loc):
+        assert new_loc in mytest.get_cache_location()
+        assert "astroquery" in mytest.get_cache_location()
+        assert mytest.name in mytest.get_cache_location()
+
+
 def test_login():
 
     mytest = TestClass()
-    assert mytest.cache_active
+    assert conf.cache_active
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0
 
     set_response(TEXT1)  # Text 1 is set as the approved password
 
     mytest.login("ceb")
     assert mytest.authenticated()
-    assert len(os.listdir(mytest.cache_location)) == 0  # request should not be cached
+    assert len(os.listdir(mytest.get_cache_location())) == 0  # request should not be cached
 
-    set_response(TEXT2)  # Text 1 is not the approved password
+    set_response(TEXT2)  # Text 2 is not the approved password
 
     mytest.login("ceb")
     assert not mytest.authenticated()  # Should not be accessing cache
@@ -116,12 +151,12 @@ def test_login():
 def test_timeout():
 
     mytest = TestClass()
-    assert mytest.cache_active
+    assert conf.cache_active
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0
 
-    mytest.cache_timeout = 2  # Set to 2 sec so we can reach timeout easily
+    conf.cache_timeout = 2  # Set to 2 sec so we can reach timeout easily
 
     set_response(TEXT1)  # setting the response
 
@@ -141,19 +176,19 @@ def test_timeout():
 def test_deactivate():
 
     mytest = TestClass()
-    mytest.cache_active = False
+    conf.cache_active = False
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0
 
     set_response(TEXT1)
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT1
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0
 
     set_response(TEXT2)
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT2
-    assert len(os.listdir(mytest.cache_location)) == 0
+    assert len(os.listdir(mytest.get_cache_location())) == 0