making new caching backwards compatible

Author: ceb8

astroquery/query.py

@@ -18,6 +18,8 @@
 import astropy.units as u
 from astropy.utils.console import ProgressBarOrSpinner
 import astropy.utils.data
+from astropy.utils import deprecated
+from astropy.utils.exceptions import AstropyDeprecationWarning
 
 from astroquery import version, log, conf
 from astroquery.utils import system_tools
@@ -106,7 +108,7 @@ def hash(self):
         return self._hash
 
     def request_file(self, cache_location):
-        fn = os.path.join(cache_location, self.hash() + ".pickle")
+        fn = cache_location.joinpath(self.hash() + ".pickle")
         return fn
 
     def from_cache(self, cache_location, cache_timeout):
@@ -116,7 +118,7 @@ def from_cache(self, cache_location, cache_timeout):
                 expired = False
             else:
                 current_time = datetime.utcnow()
-                cache_time = datetime.utcfromtimestamp(os.path.getmtime(request_file))
+                cache_time = datetime.utcfromtimestamp(request_file.stat().st_mtime)
                 expired = ((current_time-cache_time) > timedelta(seconds=cache_timeout))
             if not expired:
                 with open(request_file, "rb") as f:
@@ -139,8 +141,8 @@ def remove_cache_file(self, cache_location):
         """
         request_file = self.request_file(cache_location)
 
-        if os.path.exists(request_file):
-            os.remove(request_file)
+        if request_file.exists:
+            request_file.unlink()
         else:
             raise FileNotFoundError(f"Tried to remove cache file {request_file} but "
                                     "it does not exist")
@@ -188,7 +190,7 @@ def __init__(self):
                     olduseragent=S.headers['User-Agent']))
 
         self.name = self.__class__.__name__.split("Class")[0]
-        self.cache_location = None
+        self._cache_location = None
 
     def __call__(self, *args, **kwargs):
         """ init a fresh copy of self """
@@ -229,23 +231,23 @@ def _response_hook(self, response, *args, **kwargs):
             log.log(5, f"HTTP response\n{response_log}")
 
     @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)
+    def cache_location(self):
+        cl = self._cache_location or Path(paths.get_cache_dir(), 'astroquery', self.name)
+        cl.mkdir(parents=True, exist_ok=True)
         return cl
 
-    def get_cache_location(self):
-        return self._cache_location
+    @cache_location.setter
+    def cache_location(self, loc):
+        self._cache_location = Path(loc)
 
     def reset_cache_location(self):
-        self.cache_location = None
+        """Resets the cache location to the default astropy cache"""
+        self._cache_location = None
 
     def clear_cache(self):
         """Removes all cache files."""
-
-        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))
+        for fle in self.cache_location.glob("*.pickle"):
+            fle.unlink()
 
     def _request(self, method, url,
                  params=None, data=None, headers=None,
@@ -317,10 +319,7 @@ def _request(self, method, url,
             json=json
         )
 
-        if (cache is not False) and conf.cache_active:
-            cache = True
-        else:
-            cache = False
+        cache &= conf.cache_active
 
         if save:
             local_filename = url.split('/')[-1]
@@ -329,7 +328,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,
@@ -341,23 +340,23 @@ def _request(self, method, url,
                 return local_filepath
         else:
             query = AstroQuery(method, url, **req_kwargs)
-            if ((self._cache_location is None) or (not cache)):
+            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, conf.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
@@ -484,6 +483,8 @@ def _download_file(self, url, local_filepath, timeout=None, auth=None,
         return response
 
 
+@deprecated(since="v0.4.7", message=("The suspend_cache function is deprecated,"
+                                     "Use the conf set_temp function instead."))
 class suspend_cache:
     """
     A context manager that suspends caching.

astroquery/tests/test_cache.py

@@ -51,6 +51,8 @@ def _login(self, username):
 
 
 def test_conf():
+    conf.reset()
+
     default_timeout = conf.cache_timeout
     default_active = conf.cache_active
 
@@ -75,72 +77,75 @@ def test_conf():
 
 
 def test_basic_caching():
+    conf.reset()
 
     mytest = TestClass()
     assert conf.cache_active
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.cache_location)) == 0
 
     set_response(TEXT1)
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT1
-    assert len(os.listdir(mytest.get_cache_location())) == 1
+    assert len(os.listdir(mytest.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.get_cache_location())) == 2
+    assert len(os.listdir(mytest.cache_location)) == 2
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT1  # query that was cached
-    assert len(os.listdir(mytest.get_cache_location())) == 2  # no new cache file
+    assert len(os.listdir(mytest.cache_location)) == 2  # no new cache file
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.cache_location)) == 0
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT2  # Now get new response
 
 
 def test_change_location(tmpdir):
+    conf.reset()
 
     mytest = TestClass()
-    default_cache_location = mytest.get_cache_location()
+    default_cache_location = mytest.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()
+    assert paths.get_cache_dir() in str(default_cache_location)
+    assert "astroquery" in mytest.cache_location.parts
+    assert mytest.name in mytest.cache_location.parts
 
-    new_loc = os.path.join(tmpdir, "new_dir")
+    new_loc = "new_dir"
     mytest.cache_location = new_loc
-    assert mytest.get_cache_location() == new_loc
+    assert str(mytest.cache_location) == new_loc
 
     mytest.reset_cache_location()
-    assert mytest.get_cache_location() == default_cache_location
+    assert mytest.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()
+        assert new_loc in mytest.cache_location.parts
+        assert "astroquery" in mytest.cache_location.parts
+        assert mytest.name in mytest.cache_location.parts
 
 
 def test_login():
+    conf.reset()
 
     mytest = TestClass()
     assert conf.cache_active
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.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.get_cache_location())) == 0  # request should not be cached
+    assert len(os.listdir(mytest.cache_location)) == 0  # request should not be cached
 
     set_response(TEXT2)  # Text 2 is not the approved password
 
@@ -149,12 +154,13 @@ def test_login():
 
 
 def test_timeout():
+    conf.reset()
 
     mytest = TestClass()
     assert conf.cache_active
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.cache_location)) == 0
 
     conf.cache_timeout = 2  # Set to 2 sec so we can reach timeout easily
 
@@ -174,21 +180,43 @@ def test_timeout():
 
 
 def test_deactivate():
+    conf.reset()
 
     mytest = TestClass()
     conf.cache_active = False
 
     mytest.clear_cache()
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.cache_location)) == 0
 
     set_response(TEXT1)
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT1
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.cache_location)) == 0
 
     set_response(TEXT2)
 
     resp = mytest.test_func(URL1)
     assert resp.content == TEXT2
-    assert len(os.listdir(mytest.get_cache_location())) == 0
+    assert len(os.listdir(mytest.cache_location)) == 0
+
+    conf.reset()
+    assert conf.cache_active is True
+
+    with conf.set_temp('cache_active', False):
+        mytest.clear_cache()
+        assert len(os.listdir(mytest.cache_location)) == 0
+
+        set_response(TEXT1)
+
+        resp = mytest.test_func(URL1)
+        assert resp.content == TEXT1
+        assert len(os.listdir(mytest.cache_location)) == 0
+
+        set_response(TEXT2)
+
+        resp = mytest.test_func(URL1)
+        assert resp.content == TEXT2
+        assert len(os.listdir(mytest.cache_location)) == 0
+
+    assert conf.cache_active is True