Merge pull request #2836 from bsipocz/ENH_basevoquery

ENH: adding minimalist BaseVOQuery baseclass

Author: bsipocz

CHANGES.rst

@@ -301,7 +301,9 @@ Infrastructure, Utility and Other Changes and Additions
 - New function, ``utils.cleanup_downloads.cleanup_saved_downloads``, is
   added to help the testcleanup narrative in narrative documentations. [#2384]
 
-- Adding more system information to User-Agent. [#2762]
+- Adding new ``BaseVOQuery`` baseclass for modules using VO tools. [#2836]
+
+- Adding more system and package information to User-Agent. [#2762, #2836]
 
 - Removal of the non-functional ``nrao`` module as it was completely
   incompatible with the refactored upstream API. [#2546]

astroquery/alma/core.py

@@ -30,7 +30,7 @@
 from ..exceptions import LoginError
 from ..utils import commons
 from ..utils.process_asyncs import async_to_sync
-from ..query import BaseQuery, QueryWithLogin
+from ..query import BaseQuery, QueryWithLogin, BaseVOQuery
 from .tapsql import (_gen_pos_sql, _gen_str_sql, _gen_numeric_sql,
                      _gen_band_list_sql, _gen_datetime_sql, _gen_pol_sql, _gen_pub_sql,
                      _gen_science_sql, _gen_spec_res_sql, ALMA_DATE_FORMAT)
@@ -212,7 +212,7 @@ def _gen_sql(payload):
     return sql + where
 
 
-class AlmaAuth(BaseQuery):
+class AlmaAuth(BaseVOQuery, BaseQuery):
     """Authentication session information for passing credentials to an OIDC instance
 
     Assumes an OIDC system like Keycloak with a preconfigured client app called "oidc" to validate against.
@@ -366,7 +366,7 @@ def sia_url(self):
     @property
     def tap(self):
         if not self._tap:
-            self._tap = pyvo.dal.tap.TAPService(baseurl=self.tap_url)
+            self._tap = pyvo.dal.tap.TAPService(baseurl=self.tap_url, session=self._session)
         return self._tap
 
     @property

astroquery/cadc/core.py

@@ -17,7 +17,7 @@
 
 from ..utils.class_or_instance import class_or_instance
 from ..utils import async_to_sync, commons
-from ..query import BaseQuery
+from ..query import BaseQuery, BaseVOQuery
 from bs4 import BeautifulSoup
 from astropy import units as u
 from astropy.coordinates import Angle
@@ -37,7 +37,7 @@
 
 
 @async_to_sync
-class CadcClass(BaseQuery):
+class CadcClass(BaseVOQuery, BaseQuery):
     """
     Class for accessing CADC data. Typical usage:
 

astroquery/ipac/irsa/core.py

@@ -13,7 +13,7 @@
 from astropy.utils.decorators import deprecated_renamed_argument
 from pyvo.dal import TAPService
 from astroquery import log
-from astroquery.query import BaseQuery
+from astroquery.query import BaseVOQuery
 from astroquery.utils.commons import parse_coordinates
 from astroquery.ipac.irsa import conf
 from astroquery.exceptions import InvalidQueryError
@@ -22,7 +22,7 @@
 __all__ = ['Irsa', 'IrsaClass']
 
 
-class IrsaClass(BaseQuery):
+class IrsaClass(BaseVOQuery):
 
     def __init__(self):
         super().__init__()
@@ -32,7 +32,7 @@ def __init__(self):
     @property
     def tap(self):
         if not self._tap:
-            self._tap = TAPService(baseurl=self.tap_url)
+            self._tap = TAPService(baseurl=self.tap_url, session=self._session)
         return self._tap
 
     def query_tap(self, query, *, maxrec=None):

astroquery/ipac/nexsci/nasa_exoplanet_archive/core.py

@@ -21,7 +21,7 @@
 
 # Import astroquery utilities
 from astroquery.exceptions import InputWarning, InvalidQueryError, NoResultsWarning, RemoteServiceError
-from astroquery.query import BaseQuery
+from astroquery.query import BaseQuery, BaseVOQuery
 from astroquery.utils import async_to_sync, commons
 from astroquery.utils.class_or_instance import class_or_instance
 from astroquery.ipac.nexsci.nasa_exoplanet_archive import conf
@@ -129,7 +129,7 @@ class InvalidTableError(InvalidQueryError):
 # Class decorator, async_to_sync, modifies NasaExoplanetArchiveClass to convert
 # all query_x_async methods to query_x methods
 @async_to_sync
-class NasaExoplanetArchiveClass(BaseQuery):
+class NasaExoplanetArchiveClass(BaseVOQuery, BaseQuery):
     """
     The interface for querying the NASA Exoplanet Archive TAP and API services
 
@@ -230,7 +230,7 @@ def query_criteria_async(self, table, get_query_payload=False, cache=None, **cri
             cache = self.CACHE
 
         if table in self.TAP_TABLES:
-            tap = pyvo.dal.tap.TAPService(baseurl=self.URL_TAP)
+            tap = pyvo.dal.tap.TAPService(baseurl=self.URL_TAP, session=self._session)
             # construct query from table and request_payload (including format)
             tap_query = self._request_to_sql(request_payload)
 

astroquery/query.py

@@ -21,11 +21,13 @@
 import astropy.utils.data
 from astropy.utils import deprecated
 
+import pyvo
+
 from astroquery import version, log, cache_conf
 from astroquery.utils import system_tools
 
 
-__all__ = ['BaseQuery', 'QueryWithLogin']
+__all__ = ['BaseVOQuery', 'BaseQuery', 'QueryWithLogin']
 
 
 def to_cache(response, cache_file):
@@ -175,18 +177,45 @@ def login(*args, **kwargs):
         return newcls
 
 
+class BaseVOQuery:
+    """
+    Bare minimum base query that sets the Session header to include both astroquery and pyvo.
+    Use in modules that rely on PyVO, either on its own or in combination with ``BaseQuery`` (be mindful
+    about resolution order of base classes!).
+    """
+    def __init__(self):
+        super().__init__()
+        if not hasattr(self, '_session'):
+            # We don't want to override another, e.g. already authenticated session from another baseclass
+            self._session = requests.Session()
+
+        user_agents = self._session.headers['User-Agent'].split()
+        if 'astroquery' in user_agents[0]:
+            if 'pyVO' not in user_agents[1]:
+                user_agents[0] = f"astroquery/{version.version} pyVO/{pyvo.__version__}"
+        elif 'pyVO' in user_agents[0]:
+            user_agents[0] = f"astroquery/{version.version} pyVO/{pyvo.__version__}"
+        else:
+            user_agents = [f"astroquery/{version.version} pyVO/{pyvo.__version__} "
+                           f"Python/{platform.python_version()} ({platform.system()})"] + user_agents
+
+        self._session.headers['User-Agent'] = " ".join(user_agents)
+
+        self.name = self.__class__.__name__.split("Class")[0]
+
+
 class BaseQuery(metaclass=LoginABCMeta):
     """
     This is the base class for all the query classes in astroquery. It
     is implemented as an abstract class and must not be directly instantiated.
     """
 
     def __init__(self):
-        S = self._session = requests.Session()
+        self._session = requests.Session()
         self._session.hooks['response'].append(self._response_hook)
-        S.headers['User-Agent'] = (
+        self._session.headers['User-Agent'] = (
             f"astroquery/{version.version} Python/{platform.python_version()} ({platform.system()}) "
-            f"{S.headers['User-Agent']}")
+            f"{self._session.headers['User-Agent']}")
 
         self.name = self.__class__.__name__.split("Class")[0]
         self._cache_location = None

astroquery/tests/test_query.py

@@ -0,0 +1,39 @@
+# Licensed under a 3-clause BSD style license - see LICENSE.rst
+from astroquery.query import BaseQuery, BaseVOQuery
+
+
+class with_VO(BaseVOQuery, BaseQuery):
+    pass
+
+
+class without_VO(BaseQuery):
+    pass
+
+
+class only_VO(BaseVOQuery):
+    pass
+
+
+def test_session_VO_header():
+    test_instance = with_VO()
+    user_agent = test_instance._session.headers['User-Agent']
+    assert 'astroquery' in user_agent
+    assert 'pyVO' in user_agent
+    assert user_agent.count('astroquery') == 1
+
+
+def test_session_nonVO_header():
+    test_instance = without_VO()
+    user_agent = test_instance._session.headers['User-Agent']
+    assert 'astroquery' in user_agent
+    assert 'pyVO' not in user_agent
+    assert user_agent.count('astroquery') == 1
+
+
+def test_session_hooks():
+    # Test that we don't override the session in the BaseVOQuery
+    test_instance = with_VO()
+    assert len(test_instance._session.hooks['response']) > 0
+
+    test_VO_instance = only_VO()
+    assert len(test_VO_instance._session.hooks['response']) == 0