implementing review suggestions

astroquery/__init__.py

@@ -45,8 +45,8 @@ def _get_bibtex():
 class Cache_Conf(_config.ConfigNamespace):
 
     cache_timeout = _config.ConfigItem(
-        604800,  # 1 week
-        'Astroquery-wide cache timeout (seconds).',
+        604800,
+        'Astroquery-wide cache timeout (seconds). Default is 1 week (604800).',
         cfgtype='integer'
     )
 

astroquery/query.py

@@ -119,14 +119,14 @@ def from_cache(self, cache_location, cache_timeout):
             else:
                 current_time = datetime.utcnow()
                 cache_time = datetime.utcfromtimestamp(request_file.stat().st_mtime)
-                expired = ((current_time-cache_time) > timedelta(seconds=cache_timeout))
+                expired = current_time-cache_time > timedelta(seconds=cache_timeout)
             if not expired:
                 with open(request_file, "rb") as f:
                     response = pickle.load(f)
                 if not isinstance(response, requests.Response):
                     response = None
             else:
-                log.debug("Cache expired for {0}...".format(request_file))
+                log.debug(f"Cache expired for {request_file}...")
                 response = None
         except FileNotFoundError:
             response = None
@@ -251,7 +251,7 @@ def clear_cache(self):
 
     def _request(self, method, url,
                  params=None, data=None, headers=None,
-                 files=None, save=False, savedir='', timeout=None, cache=True,
+                 files=None, save=False, savedir='', timeout=None, cache=None,
                  stream=False, auth=None, continuation=True, verify=True,
                  allow_redirects=True,
                  json=None, return_response_on_save=False):
@@ -311,7 +311,8 @@ def _request(self, method, url,
             is True.
         """
 
-        cache &= cache_conf.cache_active
+        if cache is None:  # Global caching not overridden
+            cache = cache_conf.cache_active
 
         if save:
             local_filename = url.split('/')[-1]
@@ -334,7 +335,7 @@ def _request(self, method, url,
         else:
             query = AstroQuery(method, url, params=params, data=data, headers=headers,
                                files=files, timeout=timeout, json=json)
-            if (self.cache_location is None) or (not cache):
+            if not cache:
                 with cache_conf.set_temp("cache_active", False):
                     response = query.request(self._session, stream=stream,
                                              auth=auth, verify=verify,
@@ -488,10 +489,10 @@ def __init__(self, obj=None):
         self.original_cache_setting = cache_conf.cache_active
 
     def __enter__(self):
-        conf.cache_active = False
+        cache_conf.cache_active = False
 
     def __exit__(self, exc_type, exc_value, traceback):
-        conf.cache_active = self.original_cache_setting
+        cache_conf.cache_active = self.original_cache_setting
         return False
 
 
@@ -547,7 +548,7 @@ def _login(self, *args, **kwargs):
         pass
 
     def login(self, *args, **kwargs):
-        with conf.set_temp("cache_active", False):
+        with cache_conf.set_temp("cache_active", False):
             self._authenticated = self._login(*args, **kwargs)
         return self._authenticated
 

astroquery/tests/test_cache.py

@@ -8,7 +8,7 @@
 from astropy.config import paths
 
 from astroquery.query import QueryWithLogin
-from astroquery import conf
+from astroquery import cache_conf
 
 URL1 = "http://fakeurl.edu"
 URL2 = "http://fakeurl.ac.uk"
@@ -33,7 +33,7 @@ def get_mockreturn(url, *args, **kwargs):
     requests.Session.request = get_mockreturn
 
 
-class TestClass(QueryWithLogin):
+class CacheTestClass(QueryWithLogin):
     """Bare bones class for testing caching"""
 
     def test_func(self, requrl):
@@ -42,45 +42,40 @@ def test_func(self, requrl):
 
     def _login(self, username):
 
-        resp = self._request(method="GET", url=username)
-
-        if resp.content == "Penguin":
-            return True
-        else:
-            return False
+        return self._request(method="GET", url=username).content == "Penguin"
 
 
 def test_conf():
-    conf.reset()
+    cache_conf.reset()
 
-    default_timeout = conf.cache_timeout
-    default_active = conf.cache_active
+    default_timeout = cache_conf.cache_timeout
+    default_active = cache_conf.cache_active
 
     assert default_timeout == 604800
     assert default_active is True
 
-    with conf.set_temp("cache_timeout", 5):
-        assert conf.cache_timeout == 5
+    with cache_conf.set_temp("cache_timeout", 5):
+        assert cache_conf.cache_timeout == 5
 
-    with conf.set_temp("cache_active", False):
-        assert conf.cache_active is False
+    with cache_conf.set_temp("cache_active", False):
+        assert cache_conf.cache_active is False
 
-    assert conf.cache_timeout == default_timeout
-    assert conf.cache_active == default_active
+    assert cache_conf.cache_timeout == default_timeout
+    assert cache_conf.cache_active == default_active
 
-    conf.cache_timeout = 5
-    conf.cache_active = False
-    conf.reset()
+    cache_conf.cache_timeout = 5
+    cache_conf.cache_active = False
+    cache_conf.reset()
 
-    assert conf.cache_timeout == default_timeout
-    assert conf.cache_active == default_active
+    assert cache_conf.cache_timeout == default_timeout
+    assert cache_conf.cache_active == default_active
 
 
 def test_basic_caching():
-    conf.reset()
+    cache_conf.reset()
 
-    mytest = TestClass()
-    assert conf.cache_active
+    mytest = CacheTestClass()
+    assert cache_conf.cache_active
 
     mytest.clear_cache()
     assert len(os.listdir(mytest.cache_location)) == 0
@@ -108,35 +103,35 @@ def test_basic_caching():
     assert resp.content == TEXT2  # Now get new response
 
 
-def test_change_location(tmpdir):
-    conf.reset()
+def test_change_location(tmp_path):
+    cache_conf.reset()
 
-    mytest = TestClass()
+    mytest = CacheTestClass()
     default_cache_location = mytest.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 = "new_dir"
+    new_loc = tmp_path.joinpath("new_dir")
     mytest.cache_location = new_loc
-    assert str(mytest.cache_location) == new_loc
+    assert mytest.cache_location == new_loc
 
     mytest.reset_cache_location()
     assert mytest.cache_location == default_cache_location
 
-    Path(new_loc).mkdir(parents=True, exist_ok=True)
+    new_loc.mkdir(parents=True, exist_ok=True)
     with paths.set_temp_cache(new_loc):
-        assert new_loc in mytest.cache_location.parts
+        assert str(new_loc) in str(mytest.cache_location)
         assert "astroquery" in mytest.cache_location.parts
         assert mytest.name in mytest.cache_location.parts
 
 
 def test_login():
-    conf.reset()
+    cache_conf.reset()
 
-    mytest = TestClass()
-    assert conf.cache_active
+    mytest = CacheTestClass()
+    assert cache_conf.cache_active
 
     mytest.clear_cache()
     assert len(os.listdir(mytest.cache_location)) == 0
@@ -154,15 +149,15 @@ def test_login():
 
 
 def test_timeout():
-    conf.reset()
+    cache_conf.reset()
 
-    mytest = TestClass()
-    assert conf.cache_active
+    mytest = CacheTestClass()
+    assert cache_conf.cache_active
 
     mytest.clear_cache()
     assert len(os.listdir(mytest.cache_location)) == 0
 
-    conf.cache_timeout = 2  # Set to 2 sec so we can reach timeout easily
+    cache_conf.cache_timeout = 2  # Set to 2 sec so we can reach timeout easily
 
     set_response(TEXT1)  # setting the response
 
@@ -180,10 +175,10 @@ def test_timeout():
 
 
 def test_deactivate():
-    conf.reset()
+    cache_conf.reset()
 
-    mytest = TestClass()
-    conf.cache_active = False
+    mytest = CacheTestClass()
+    cache_conf.cache_active = False
 
     mytest.clear_cache()
     assert len(os.listdir(mytest.cache_location)) == 0
@@ -200,10 +195,10 @@ def test_deactivate():
     assert resp.content == TEXT2
     assert len(os.listdir(mytest.cache_location)) == 0
 
-    conf.reset()
-    assert conf.cache_active is True
+    cache_conf.reset()
+    assert cache_conf.cache_active is True
 
-    with conf.set_temp('cache_active', False):
+    with cache_conf.set_temp('cache_active', False):
         mytest.clear_cache()
         assert len(os.listdir(mytest.cache_location)) == 0
 
@@ -219,4 +214,4 @@ def test_deactivate():
         assert resp.content == TEXT2
         assert len(os.listdir(mytest.cache_location)) == 0
 
-    assert conf.cache_active is True
+    assert cache_conf.cache_active is True

docs/index.rst

@@ -176,39 +176,47 @@ uncomment the relevant configuration item(s), and insert your desired value(s).
 Caching
 -------
 
-By default Astroquery employs query caching with a timeout of 1 week, although individual services
-may have different settings. The user can clear their cache at any time, as well as suspend cache usage,
+By default Astroquery employs query caching with a timeout of 1 week.
+The user can clear their cache at any time, as well as suspend cache usage,
 and change the cache location. Caching persists between Astroquery sessions.
 If you know the service you are using has released new data recently, or if you believe you are
 not recieving the newest data, try clearing the cache.
 
 
 The Astroquery cache location is divided by service, so each service's cache should be managed invidually,
 however whether the cache is active and the expiration time are controlled centrally through the
-astroquery ``conf`` module. Astroquery uses the Astropy configuration infrastructure, information about
+astroquery ``cache_conf`` module. Astroquery uses the Astropy configuration infrastructure, information about
 temporarily or permanently changing configuration values can be found
 `here <https://docs.astropy.org/en/latest/config/index.html>`_.
 
 Shown here are the cache properties, using Simbad as an example:
 
 .. code-block:: python
 
-  >>> from astroquery import conf
+  >>> from astroquery import cache_conf
   >>> from astroquery.simbad import Simbad
 
   >>> # Is the cache active?
-  >>> print(conf.cache_active)
+  >>> print(cache_conf.cache_active)
   True
 
   >>> # Cache timout in seconds
-  >>> print(conf.cache_timeout)
+  >>> print(cache_conf.cache_timeout)
   604800
   
   >>> # Cache location
   >>> print(Simbad.cache_location)
   /Users/username/.astropy/cache/astroquery/Simbad
 
 
+To clear the cache:
+
+.. code-block:: python
+
+    >>> Simbad.clear_cache()
+
+
+
 
 Available Services
 ==================