From 81197c3263a2c682f16a797b7e6093596ec403e5 Mon Sep 17 00:00:00 2001 From: James Dempsey Date: Sat, 30 Apr 2022 16:02:37 +1000 Subject: [PATCH 1/6] Switch CASDA to QueryWithLogin --- astroquery/casda/__init__.py | 11 +- astroquery/casda/core.py | 72 ++++++++-- astroquery/casda/tests/data/availability.xml | 6 + astroquery/casda/tests/test_casda.py | 133 +++++++++++++++---- astroquery/casda/tests/test_casda_remote.py | 32 +++-- docs/casda/casda.rst | 27 ++-- 6 files changed, 215 insertions(+), 66 deletions(-) create mode 100644 astroquery/casda/tests/data/availability.xml diff --git a/astroquery/casda/__init__.py b/astroquery/casda/__init__.py index 0172f44915..f9f5323a2f 100644 --- a/astroquery/casda/__init__.py +++ b/astroquery/casda/__init__.py @@ -11,7 +11,7 @@ class Conf(_config.ConfigNamespace): server = _config.ConfigItem( ['https://casda.csiro.au/casda_vo_tools/sia2/query'], - 'Name of the CASDA SIA server to use.' + 'Address of the CASDA SIA server to use.' ) timeout = _config.ConfigItem( 30, @@ -25,7 +25,14 @@ class Conf(_config.ConfigNamespace): ['https://casda.csiro.au/casda_data_access/'], 'Address of the CASDA SODA server' ) - + login_url = _config.ConfigItem( + ['https://data.csiro.au/casda_vo_proxy/vo/tap/availability'], + 'Address for test logins' + ) + username = _config.ConfigItem( + '', + 'Optional default username for CASDA archive.' + ) conf = Conf() diff --git a/astroquery/casda/core.py b/astroquery/casda/core.py index 6d9b074f9c..08dbc5a81a 100644 --- a/astroquery/casda/core.py +++ b/astroquery/casda/core.py @@ -7,6 +7,7 @@ import time from xml.etree import ElementTree from datetime import datetime, timezone +import keyring # 2. third party imports import astropy.units as u @@ -17,10 +18,7 @@ import numpy as np # 3. local imports - use relative imports -# commonly required local imports shown below as example -# all Query classes should inherit from BaseQuery. -from ..query import BaseQuery -# has common functions required by most modules +from ..query import QueryWithLogin from ..utils import commons # prepend_docstr is a way to copy docstrings between methods from ..utils import prepend_docstr_nosections @@ -28,13 +26,14 @@ from ..utils import async_to_sync # import configurable items declared in __init__.py from . import conf +from ..exceptions import LoginError # export all the public classes and methods __all__ = ['Casda', 'CasdaClass'] @async_to_sync -class CasdaClass(BaseQuery): +class CasdaClass(QueryWithLogin): """ Class for accessing ASKAP data through the CSIRO ASKAP Science Data Archive (CASDA). Typical usage: @@ -46,18 +45,69 @@ class CasdaClass(BaseQuery): URL = conf.server TIMEOUT = conf.timeout POLL_INTERVAL = conf.poll_interval + USERNAME = conf.username _soda_base_url = conf.soda_base_url + _login_url =conf.login_url _uws_ns = {'uws': 'http://www.ivoa.net/xml/UWS/v1.0'} def __init__(self, user=None, password=None): super().__init__() - if user is None: - self._authenticated = False + + def _login(self, *, username=None, store_password=False, + reenter_password=False, password=None): + """ + login to non-public data as a known user + + Parameters + ---------- + username : str, optional + Username to the CASDA archive, uses ATNF OPAL credentials. If not given, it should be + specified in the config file. + store_password : bool, optional + Stores the password securely in your keyring. Default is False. + reenter_password : bool, optional + Asks for the password even if it is already stored in the + keyring. This is the way to overwrite an already stored passwork + on the keyring. Default is False. + password : str, optional + Password for the CASDA archive. If not given, it will obtained either from the keyring or interactively. + """ + + if username is None: + if not self.USERNAME: + raise LoginError("If you do not pass a username to login(), " + "you should configure a default one!") + else: + username = self.USERNAME + + if password is None: + # Get password from keyring or prompt + password, password_from_keyring = self._get_password( + "astroquery:casda.csiro.au", username, reenter=reenter_password) else: - self._authenticated = True - # self._user = user - # self._password = password - self._auth = (user, password) + # Bypass the keyring if a password is supplied + password_from_keyring = None + store_password = False + + # Login to CASDA to test credentals + log.info("Authenticating {0} on CASDA ...".format(username)) + auth = (username, password) + login_response = self._request("GET", self._login_url, auth=auth, + timeout=self.TIMEOUT, cache=False) + authenticated = login_response.status_code == 200 + + if authenticated: + log.info("Authentication successful!") + self.USERNAME = username + self._auth = (username, password) + + # When authenticated, save password in keyring if needed + if password_from_keyring is None and store_password: + keyring.set_password("astroquery:casda.csiro.au", username, password) + else: + log.exception("Authentication failed") + + return authenticated def query_region_async(self, coordinates, radius=1*u.arcmin, height=None, width=None, get_query_payload=False, cache=True): diff --git a/astroquery/casda/tests/data/availability.xml b/astroquery/casda/tests/data/availability.xml new file mode 100644 index 0000000000..d518b2bdfe --- /dev/null +++ b/astroquery/casda/tests/data/availability.xml @@ -0,0 +1,6 @@ + + + true + 2022-04-07T11:11:40.006Z + + \ No newline at end of file diff --git a/astroquery/casda/tests/test_casda.py b/astroquery/casda/tests/test_casda.py index 3ac298f1dc..4c3b815a1a 100644 --- a/astroquery/casda/tests/test_casda.py +++ b/astroquery/casda/tests/test_casda.py @@ -2,10 +2,10 @@ # Licensed under a 3-clause BSD style license - see LICENSE.rst - import pytest import requests import os +import keyring from astropy.coordinates import SkyCoord import astropy.units as u @@ -15,6 +15,7 @@ import numpy as np from astroquery.casda import Casda +from astroquery.exceptions import LoginError try: from unittest.mock import Mock, patch, MagicMock @@ -23,14 +24,18 @@ DATA_FILES = {'CIRCLE': 'cone.xml', 'RANGE': 'box.xml', 'DATALINK': 'datalink.xml', 'RUN_JOB': 'run_job.xml', 'COMPLETED_JOB': 'completed_job.xml', 'DATALINK_NOACCESS': 'datalink_noaccess.xml', - 'cutout_CIRCLE_333.9092_-45.8418_0.5000': 'cutout_333.9092_-45.8418_0.5000.xml'} + 'cutout_CIRCLE_333.9092_-45.8418_0.5000': 'cutout_333.9092_-45.8418_0.5000.xml', + 'AVAILABILITY' : 'availability.xml'} +USERNAME='user' +PASSWORD='password' class MockResponse: def __init__(self, content): self.content = content self.text = content.decode() + self.status_code = 200 def raise_for_status(self): return @@ -41,7 +46,7 @@ def get_mockreturn(self, method, url, data=None, timeout=10, log.debug("get_mockreturn url:{} params:{} kwargs:{}".format(url, params, kwargs)) if kwargs and 'auth' in kwargs: auth = kwargs['auth'] - if auth and (auth[0] != 'user' or auth[1] != 'password'): + if auth and (auth[0] != USERNAME or auth[1] != PASSWORD): log.debug("Rejecting credentials") return create_auth_failure_response() @@ -71,6 +76,8 @@ def get_mockreturn(self, method, url, data=None, timeout=10, key = 'DATALINK' else: key = 'DATALINK_NOACCESS' + elif str(url) == 'https://data.csiro.au/casda_vo_proxy/vo/tap/availability': + key = 'AVAILABILITY' else: key = params['POS'].split()[0] if params['POS'] else None filename = data_path(DATA_FILES[key]) @@ -107,7 +114,6 @@ def patch_get(request): mp.setattr(requests.Session, 'request', get_mockreturn) return mp - def data_path(filename): data_dir = os.path.join(os.path.dirname(__file__), 'data') return os.path.join(data_dir, filename) @@ -116,6 +122,61 @@ def data_path(filename): def isclose(value1, value2, abs_tol=1e-09): return abs(value1 - value2) < abs_tol +def test_login(patch_get): + casda = Casda() + assert casda._authenticated == False + assert casda.USERNAME == '' + + casda.login(username=USERNAME, password=PASSWORD) + assert casda._authenticated == True + assert casda.USERNAME == USERNAME + assert casda._auth == (USERNAME, 'password') + +def test_login_badpassword(patch_get): + casda = Casda() + assert casda._authenticated == False + assert casda.USERNAME == '' + + casda.login(username=USERNAME, password='notthepassword') + assert casda._authenticated == False + assert casda.USERNAME == '' + assert hasattr(casda, '_auth') == False + +def test_login_default_user(patch_get): + casda = Casda() + casda.USERNAME = USERNAME + assert casda._authenticated == False + + casda.login(password=PASSWORD) + assert casda._authenticated == True + assert casda.USERNAME == USERNAME + assert casda._auth == (USERNAME, 'password') + +def test_login_no_default_user(): + casda = Casda() + assert casda._authenticated == False + assert casda.USERNAME == '' + + with pytest.raises(LoginError) as excinfo: + Casda.login() + + assert "If you do not pass a username to login()," in str(excinfo.value) + + assert casda._authenticated == False + assert casda.USERNAME == '' + assert hasattr(casda, '_auth') == False + +def test_login_keyring(patch_get): + casda = Casda() + assert casda._authenticated == False + assert casda.USERNAME == '' + keyring.set_password("astroquery:casda.csiro.au", USERNAME, PASSWORD) + + casda.login(username=USERNAME) + keyring.delete_password("astroquery:casda.csiro.au", USERNAME) + assert casda._authenticated == True + assert casda.USERNAME == USERNAME + assert casda._auth == (USERNAME, PASSWORD) def test_query_region_text_radius(patch_get): ra = 333.9092 @@ -251,7 +312,8 @@ def test_stage_data_unauthorised(patch_get): def test_stage_data_empty(patch_get): table = Table() - casda = Casda('user', 'password') + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) urls = casda.stage_data(table) assert urls == [] @@ -261,7 +323,12 @@ def test_stage_data_invalid_credentials(patch_get): access_urls = [prefix + 'cube-220'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda('user', 'notthepassword') + casda = Casda() + #Update the casda object to indicate that it has been authenticated + casda.USERNAME = USERNAME + casda._auth = (USERNAME, 'notthepassword') + casda._authenticated = True + with pytest.raises(requests.exceptions.HTTPError) as excinfo: casda.stage_data(table) @@ -270,7 +337,8 @@ def test_stage_data_no_link(patch_get): prefix = 'https://somewhere/casda/datalink/links?' access_urls = [prefix + 'cube-240'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda('user', 'password') + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) casda.POLL_INTERVAL = 1 with pytest.raises(ValueError) as excinfo: @@ -283,7 +351,8 @@ def test_stage_data(patch_get): prefix = 'https://somewhere/casda/datalink/links?' access_urls = [prefix + 'cube-244'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda('user', 'password') + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) casda.POLL_INTERVAL = 1 urls = casda.stage_data(table, verbose=True) assert urls == ['http://casda.csiro.au/download/web/111-000-111-000/askap_img.fits.checksum', @@ -299,7 +368,8 @@ def test_cutout(patch_get): radius = 30*u.arcmin centre = SkyCoord(ra, dec) - casda = Casda('user', 'password') + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) casda.POLL_INTERVAL = 1 urls = casda.cutout(table, coordinates=centre, radius=radius, verbose=True) assert urls == ['http://casda.csiro.au/download/web/111-000-111-000/cutout.fits.checksum', @@ -315,7 +385,8 @@ def test_cutout_no_args(patch_get): radius = 30*u.arcmin centre = SkyCoord(ra, dec) - casda = Casda('user', 'password') + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) casda.POLL_INTERVAL = 1 with pytest.raises(ValueError) as excinfo: casda.cutout(table) @@ -336,15 +407,19 @@ def test_cutout_unauthorised(patch_get): assert "Credentials must be supplied to download CASDA image data" in str(excinfo.value) -def test_cutout_no_table(): - casda = Casda('user', 'password') +def test_cutout_no_table(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) + casda.POLL_INTERVAL = 1 result = casda.cutout(None) assert result == [] -def test_args_to_payload_band(): - casda = Casda('user', 'password') +def test_args_to_payload_band(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) + payload = casda._args_to_payload(band=(0.195*u.m, 0.215*u.m)) assert payload['BAND'] == '0.195 0.215' assert list(payload.keys()) == ['BAND'] @@ -382,8 +457,9 @@ def test_args_to_payload_band(): assert list(payload.keys()) == ['BAND'] -def test_args_to_payload_band_invalid(): - casda = Casda('user', 'password') +def test_args_to_payload_band_invalid(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) with pytest.raises(ValueError) as excinfo: casda._args_to_payload(band='foo') @@ -410,8 +486,10 @@ def test_args_to_payload_band_invalid(): assert "Either 'channel' or 'band' values may be provided but not both." in str(excinfo.value) -def test_args_to_payload_channel(): - casda = Casda('user', 'password') +def test_args_to_payload_channel(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) + payload = casda._args_to_payload(channel=(0, 30)) assert payload['CHANNEL'] == '0 30' assert list(payload.keys()) == ['CHANNEL'] @@ -425,8 +503,9 @@ def test_args_to_payload_channel(): assert list(payload.keys()) == ['CHANNEL'] -def test_args_to_payload_channel_invalid(): - casda = Casda('user', 'password') +def test_args_to_payload_channel_invalid(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) with pytest.raises(ValueError) as excinfo: casda._args_to_payload(channel='one') @@ -445,8 +524,10 @@ def test_args_to_payload_channel_invalid(): assert "The 'channel' value must be a list of 2 integer values." in str(excinfo.value) -def test_args_to_payload_coordinates(): - casda = Casda('user', 'password') +def test_args_to_payload_coordinates(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) + cutout_coords = SkyCoord(ra=345.245*u.degree, dec=-32.125*u.degree, frame='icrs') payload = casda._args_to_payload(coordinates=cutout_coords) assert payload['POS'].startswith('CIRCLE 345') @@ -482,8 +563,9 @@ def test_args_to_payload_coordinates(): assert list(payload.keys()) == ['POS'] -def test_args_to_payload_combined(): - casda = Casda('user', 'password') +def test_args_to_payload_combined(patch_get): + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) cutout_coords = SkyCoord(ra=187.5*u.degree, dec=-60.0*u.degree, frame='icrs') payload = casda._args_to_payload(coordinates=cutout_coords, channel=(17, 23)) assert payload['POS'].startswith('CIRCLE 187') @@ -501,7 +583,8 @@ def test_download_file(patch_get): urls = ['https://ingest.pawsey.org/bucket_name/path/askap_img.fits?security=stuff', 'http://casda.csiro.au/download/web/111-000-111-000/askap_img.fits.checksum', 'https://ingest.pawsey.org.au/casda-prd-as110-01/dc52217/primary_images/RACS-DR1_0000%2B18A.fits?security=stuff'] - casda = Casda('user', 'password') + casda = Casda() + casda.login(username=USERNAME, password=PASSWORD) # skip the actual downloading of the file download_mock = MagicMock() diff --git a/astroquery/casda/tests/test_casda_remote.py b/astroquery/casda/tests/test_casda_remote.py index 2568d9eadb..dd4de7c724 100644 --- a/astroquery/casda/tests/test_casda_remote.py +++ b/astroquery/casda/tests/test_casda_remote.py @@ -2,6 +2,7 @@ # Licensed under a 3-clause BSD style license - see LICENSE.rst +import keyring import math import os import pytest @@ -39,34 +40,43 @@ def test_query_region_text_radius(self): for key in ('dataproduct_type', 'obs_id', 'access_url', 'access_format', 'obs_release_date'): assert key in responses.keys() - @pytest.mark.skipif(('CASDA_USER' not in os.environ or - 'CASDA_PASSWD' not in os.environ), + @pytest.fixture + def cached_credentials(self): + if 'CASDA_USER' in os.environ and 'CASDA_PASSWD' not in os.environ: + keyring.set_password("astroquery:casda.csiro.au", os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + yield + keyring.delete_password("astroquery:casda.csiro.au", os.environ['CASDA_USER']) + else: + yield + + @pytest.mark.skipif(('CASDA_USER' not in os.environ), reason='Requires real CASDA user/password (CASDA_USER ' - 'and CASDA_PASSWD environment variables)') - def test_stage_data(self): + 'and CASDA_PASSWD environment variables or password in keyring)') + def test_stage_data(self, cached_credentials): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-1262'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda(os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + casda = Casda() #os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + casda.login(username=os.environ['CASDA_USER']) casda.POLL_INTERVAL = 3 urls = casda.stage_data(table) - assert str(urls[0]).endswith('image_cube_g300to310.q.fits') + assert 'g300to310.q.fits' in str(urls[0]) assert str(urls[1]).endswith('image_cube_g300to310.q.fits.checksum') assert len(urls) == 2 - @pytest.mark.skipif(('CASDA_USER' not in os.environ or - 'CASDA_PASSWD' not in os.environ), + @pytest.mark.skipif(('CASDA_USER' not in os.environ), reason='Requires real CASDA user/password (CASDA_USER ' - 'and CASDA_PASSWD environment variables)') + 'and CASDA_PASSWD environment variables or password in keyring)') def test_cutout(self): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-44705'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda(os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + casda = Casda() + casda.login(username=os.environ['CASDA_USER'], password=os.environ['CASDA_PASSWD']) casda.POLL_INTERVAL = 3 pos = SkyCoord(196.49583333*u.deg, -62.7*u.deg) - urls = casda.cutout(table, pos, radius=15*u.arcmin) + urls = casda.cutout(table, coordinates=pos, radius=15*u.arcmin) # URLs may come back in any order for url in urls: diff --git a/docs/casda/casda.rst b/docs/casda/casda.rst index c29ed9ef09..bf9ae9c7ac 100644 --- a/docs/casda/casda.rst +++ b/docs/casda/casda.rst @@ -71,15 +71,14 @@ date. To change user-registration details, or to request a new OPAL password, use the link to 'Log in or reset password'. To use download tasks, you should create an instance of the ``Casda`` class -with a username and password. e.g.: +and call the :meth:`~astroquery.casda.CasdaClass.login` method with a username. The password will either be taken +from the keyring, or if in an interactive environment then it will be requested. e.g.: .. doctest-skip:: >>> from astroquery.casda import Casda - >>> import getpass - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') Data Access @@ -103,11 +102,9 @@ taken in scheduling block 2338 is shown below: >>> from astropy import coordinates, units as u, wcs >>> from astroquery.casda import Casda - >>> import getpass >>> centre = coordinates.SkyCoord.from_name('NGC 7232') - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') >>> result = Casda.query_region(centre, radius=30*u.arcmin) >>> public_data = Casda.filter_out_unreleased(result) >>> subset = public_data[(public_data['dataproduct_subtype']=='cont.restored.t0') & (public_data['obs_id']=='2338')] @@ -138,11 +135,9 @@ below: >>> from astropy import coordinates, units as u, wcs >>> from astroquery.casda import Casda - >>> import getpass >>> centre = coordinates.SkyCoord.from_name('2MASX J08161181-7039447') - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') >>> result = Casda.query_region(centre, radius=30*u.arcmin) >>> public_data = Casda.filter_out_unreleased(result) >>> subset = public_data[((public_data['obs_collection'] == 'The Rapid ASKAP Continuum Survey') & # @@ -159,11 +154,9 @@ is shown below: >>> from astropy import coordinates, units as u, wcs >>> from astroquery.casda import Casda - >>> import getpass >>> centre = coordinates.SkyCoord.from_name('NGC 1371') - >>> username = 'email@somewhere.edu.au' - >>> password = getpass.getpass(str("Enter your OPAL password: ")) - >>> casda = Casda(username, password) + >>> casda = Casda() + >>> casda.login(username='email@somewhere.edu.au') >>> result = Casda.query_region(centre, radius=30*u.arcmin) >>> public_data = Casda.filter_out_unreleased(result) >>> eridanus_cube = public_data[public_data['filename'] == 'Eridanus_full_image_V3.fits'] From 8ea211154b74ce9d4e12fb0404b298ad271afc02 Mon Sep 17 00:00:00 2001 From: James Dempsey Date: Sat, 30 Apr 2022 16:26:39 +1000 Subject: [PATCH 2/6] Tidy up and add change log entry --- CHANGES.rst | 3 ++ astroquery/casda/__init__.py | 1 + astroquery/casda/core.py | 2 +- astroquery/casda/tests/test_casda.py | 41 +++++++++++++-------- astroquery/casda/tests/test_casda_remote.py | 7 ++-- 5 files changed, 34 insertions(+), 20 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d2fc3fb30d..ccc391f5d3 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -23,6 +23,9 @@ casda - Add the ability to produce 2D and 3D cutouts from ASKAP images and cubes. [#2366] +- Use the standard ``login`` method for authenticating, which supports the system + keyring [#2386] + jplsbdb ^^^^^^^ diff --git a/astroquery/casda/__init__.py b/astroquery/casda/__init__.py index f9f5323a2f..f6ac01ca23 100644 --- a/astroquery/casda/__init__.py +++ b/astroquery/casda/__init__.py @@ -34,6 +34,7 @@ class Conf(_config.ConfigNamespace): 'Optional default username for CASDA archive.' ) + conf = Conf() from .core import Casda, CasdaClass diff --git a/astroquery/casda/core.py b/astroquery/casda/core.py index 08dbc5a81a..c37d57fc58 100644 --- a/astroquery/casda/core.py +++ b/astroquery/casda/core.py @@ -47,7 +47,7 @@ class CasdaClass(QueryWithLogin): POLL_INTERVAL = conf.poll_interval USERNAME = conf.username _soda_base_url = conf.soda_base_url - _login_url =conf.login_url + _login_url = conf.login_url _uws_ns = {'uws': 'http://www.ivoa.net/xml/UWS/v1.0'} def __init__(self, user=None, password=None): diff --git a/astroquery/casda/tests/test_casda.py b/astroquery/casda/tests/test_casda.py index 4c3b815a1a..cb5f27dde0 100644 --- a/astroquery/casda/tests/test_casda.py +++ b/astroquery/casda/tests/test_casda.py @@ -25,10 +25,11 @@ DATA_FILES = {'CIRCLE': 'cone.xml', 'RANGE': 'box.xml', 'DATALINK': 'datalink.xml', 'RUN_JOB': 'run_job.xml', 'COMPLETED_JOB': 'completed_job.xml', 'DATALINK_NOACCESS': 'datalink_noaccess.xml', 'cutout_CIRCLE_333.9092_-45.8418_0.5000': 'cutout_333.9092_-45.8418_0.5000.xml', - 'AVAILABILITY' : 'availability.xml'} + 'AVAILABILITY': 'availability.xml'} + +USERNAME = 'user' +PASSWORD = 'password' -USERNAME='user' -PASSWORD='password' class MockResponse: @@ -114,6 +115,7 @@ def patch_get(request): mp.setattr(requests.Session, 'request', get_mockreturn) return mp + def data_path(filename): data_dir = os.path.join(os.path.dirname(__file__), 'data') return os.path.join(data_dir, filename) @@ -122,39 +124,43 @@ def data_path(filename): def isclose(value1, value2, abs_tol=1e-09): return abs(value1 - value2) < abs_tol + def test_login(patch_get): casda = Casda() - assert casda._authenticated == False + assert casda._authenticated is False assert casda.USERNAME == '' casda.login(username=USERNAME, password=PASSWORD) - assert casda._authenticated == True + assert casda._authenticated is True assert casda.USERNAME == USERNAME assert casda._auth == (USERNAME, 'password') + def test_login_badpassword(patch_get): casda = Casda() - assert casda._authenticated == False + assert casda._authenticated is False assert casda.USERNAME == '' casda.login(username=USERNAME, password='notthepassword') - assert casda._authenticated == False + assert casda._authenticated is False assert casda.USERNAME == '' - assert hasattr(casda, '_auth') == False + assert hasattr(casda, '_auth') is False + def test_login_default_user(patch_get): casda = Casda() casda.USERNAME = USERNAME - assert casda._authenticated == False + assert casda._authenticated is False casda.login(password=PASSWORD) - assert casda._authenticated == True + assert casda._authenticated is True assert casda.USERNAME == USERNAME assert casda._auth == (USERNAME, 'password') + def test_login_no_default_user(): casda = Casda() - assert casda._authenticated == False + assert casda._authenticated is False assert casda.USERNAME == '' with pytest.raises(LoginError) as excinfo: @@ -162,22 +168,25 @@ def test_login_no_default_user(): assert "If you do not pass a username to login()," in str(excinfo.value) - assert casda._authenticated == False + assert casda._authenticated is False assert casda.USERNAME == '' - assert hasattr(casda, '_auth') == False + assert hasattr(casda, '_auth') is False + +@pytest.mark.skip('No keyring backend on the CI server') def test_login_keyring(patch_get): casda = Casda() - assert casda._authenticated == False + assert casda._authenticated is False assert casda.USERNAME == '' keyring.set_password("astroquery:casda.csiro.au", USERNAME, PASSWORD) casda.login(username=USERNAME) keyring.delete_password("astroquery:casda.csiro.au", USERNAME) - assert casda._authenticated == True + assert casda._authenticated is True assert casda.USERNAME == USERNAME assert casda._auth == (USERNAME, PASSWORD) + def test_query_region_text_radius(patch_get): ra = 333.9092 dec = -45.8418 @@ -324,7 +333,7 @@ def test_stage_data_invalid_credentials(patch_get): table = Table([Column(data=access_urls, name='access_url')]) casda = Casda() - #Update the casda object to indicate that it has been authenticated + # Update the casda object to indicate that it has been authenticated casda.USERNAME = USERNAME casda._auth = (USERNAME, 'notthepassword') casda._authenticated = True diff --git a/astroquery/casda/tests/test_casda_remote.py b/astroquery/casda/tests/test_casda_remote.py index dd4de7c724..388cc1d9c0 100644 --- a/astroquery/casda/tests/test_casda_remote.py +++ b/astroquery/casda/tests/test_casda_remote.py @@ -56,7 +56,7 @@ def test_stage_data(self, cached_credentials): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-1262'] table = Table([Column(data=access_urls, name='access_url')]) - casda = Casda() #os.environ['CASDA_USER'], os.environ['CASDA_PASSWD']) + casda = Casda() casda.login(username=os.environ['CASDA_USER']) casda.POLL_INTERVAL = 3 urls = casda.stage_data(table) @@ -65,9 +65,10 @@ def test_stage_data(self, cached_credentials): assert str(urls[1]).endswith('image_cube_g300to310.q.fits.checksum') assert len(urls) == 2 - @pytest.mark.skipif(('CASDA_USER' not in os.environ), + @pytest.mark.skipif(('CASDA_USER' not in os.environ or + 'CASDA_PASSWD' not in os.environ), reason='Requires real CASDA user/password (CASDA_USER ' - 'and CASDA_PASSWD environment variables or password in keyring)') + 'and CASDA_PASSWD environment variables)') def test_cutout(self): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-44705'] From 284a3d2460dec1687eebedf23336f3dad378a871 Mon Sep 17 00:00:00 2001 From: James Dempsey Date: Sat, 30 Apr 2022 16:58:11 +1000 Subject: [PATCH 3/6] Add default cutout radius --- astroquery/casda/core.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/astroquery/casda/core.py b/astroquery/casda/core.py index c37d57fc58..d7867765d5 100644 --- a/astroquery/casda/core.py +++ b/astroquery/casda/core.py @@ -324,7 +324,7 @@ def stage_data(self, table, verbose=False): return self._complete_job(job_url, verbose) - def cutout(self, table, *, coordinates=None, radius=None, height=None, + def cutout(self, table, *, coordinates=None, radius=1*u.arcmin, height=None, width=None, band=None, channel=None, verbose=False): """ Produce a cutout from each selected file. All requests for data must use authentication. If you have access to From 2c6ffb7a0e09ba9d59574a6681985bd8c06163f3 Mon Sep 17 00:00:00 2001 From: James Dempsey Date: Wed, 4 May 2022 18:33:49 +1000 Subject: [PATCH 4/6] Remove password parameter --- astroquery/casda/core.py | 17 ++---- astroquery/casda/tests/test_casda.py | 61 ++++++--------------- astroquery/casda/tests/test_casda_remote.py | 9 ++- 3 files changed, 26 insertions(+), 61 deletions(-) diff --git a/astroquery/casda/core.py b/astroquery/casda/core.py index d7867765d5..c92b369bc2 100644 --- a/astroquery/casda/core.py +++ b/astroquery/casda/core.py @@ -50,11 +50,11 @@ class CasdaClass(QueryWithLogin): _login_url = conf.login_url _uws_ns = {'uws': 'http://www.ivoa.net/xml/UWS/v1.0'} - def __init__(self, user=None, password=None): + def __init__(self): super().__init__() def _login(self, *, username=None, store_password=False, - reenter_password=False, password=None): + reenter_password=False): """ login to non-public data as a known user @@ -69,8 +69,6 @@ def _login(self, *, username=None, store_password=False, Asks for the password even if it is already stored in the keyring. This is the way to overwrite an already stored passwork on the keyring. Default is False. - password : str, optional - Password for the CASDA archive. If not given, it will obtained either from the keyring or interactively. """ if username is None: @@ -80,14 +78,9 @@ def _login(self, *, username=None, store_password=False, else: username = self.USERNAME - if password is None: - # Get password from keyring or prompt - password, password_from_keyring = self._get_password( - "astroquery:casda.csiro.au", username, reenter=reenter_password) - else: - # Bypass the keyring if a password is supplied - password_from_keyring = None - store_password = False + # Get password from keyring or prompt + password, password_from_keyring = self._get_password( + "astroquery:casda.csiro.au", username, reenter=reenter_password) # Login to CASDA to test credentals log.info("Authenticating {0} on CASDA ...".format(username)) diff --git a/astroquery/casda/tests/test_casda.py b/astroquery/casda/tests/test_casda.py index cb5f27dde0..de09b8aa16 100644 --- a/astroquery/casda/tests/test_casda.py +++ b/astroquery/casda/tests/test_casda.py @@ -125,37 +125,10 @@ def isclose(value1, value2, abs_tol=1e-09): return abs(value1 - value2) < abs_tol -def test_login(patch_get): - casda = Casda() - assert casda._authenticated is False - assert casda.USERNAME == '' - - casda.login(username=USERNAME, password=PASSWORD) - assert casda._authenticated is True - assert casda.USERNAME == USERNAME - assert casda._auth == (USERNAME, 'password') - - -def test_login_badpassword(patch_get): - casda = Casda() - assert casda._authenticated is False - assert casda.USERNAME == '' - - casda.login(username=USERNAME, password='notthepassword') - assert casda._authenticated is False - assert casda.USERNAME == '' - assert hasattr(casda, '_auth') is False - - -def test_login_default_user(patch_get): - casda = Casda() - casda.USERNAME = USERNAME - assert casda._authenticated is False - - casda.login(password=PASSWORD) - assert casda._authenticated is True - assert casda.USERNAME == USERNAME - assert casda._auth == (USERNAME, 'password') +def fake_login(casda, username, password): + casda.USERNAME = username + casda._auth = (username, password) + casda._authenticated = True def test_login_no_default_user(): @@ -322,7 +295,7 @@ def test_stage_data_unauthorised(patch_get): def test_stage_data_empty(patch_get): table = Table() casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) urls = casda.stage_data(table) assert urls == [] @@ -347,7 +320,7 @@ def test_stage_data_no_link(patch_get): access_urls = [prefix + 'cube-240'] table = Table([Column(data=access_urls, name='access_url')]) casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 with pytest.raises(ValueError) as excinfo: @@ -361,7 +334,7 @@ def test_stage_data(patch_get): access_urls = [prefix + 'cube-244'] table = Table([Column(data=access_urls, name='access_url')]) casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 urls = casda.stage_data(table, verbose=True) assert urls == ['http://casda.csiro.au/download/web/111-000-111-000/askap_img.fits.checksum', @@ -378,7 +351,7 @@ def test_cutout(patch_get): centre = SkyCoord(ra, dec) casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 urls = casda.cutout(table, coordinates=centre, radius=radius, verbose=True) assert urls == ['http://casda.csiro.au/download/web/111-000-111-000/cutout.fits.checksum', @@ -395,7 +368,7 @@ def test_cutout_no_args(patch_get): centre = SkyCoord(ra, dec) casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 with pytest.raises(ValueError) as excinfo: casda.cutout(table) @@ -418,7 +391,7 @@ def test_cutout_unauthorised(patch_get): def test_cutout_no_table(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 result = casda.cutout(None) @@ -427,7 +400,7 @@ def test_cutout_no_table(patch_get): def test_args_to_payload_band(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) payload = casda._args_to_payload(band=(0.195*u.m, 0.215*u.m)) assert payload['BAND'] == '0.195 0.215' @@ -468,7 +441,7 @@ def test_args_to_payload_band(patch_get): def test_args_to_payload_band_invalid(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) with pytest.raises(ValueError) as excinfo: casda._args_to_payload(band='foo') @@ -497,7 +470,7 @@ def test_args_to_payload_band_invalid(patch_get): def test_args_to_payload_channel(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) payload = casda._args_to_payload(channel=(0, 30)) assert payload['CHANNEL'] == '0 30' @@ -514,7 +487,7 @@ def test_args_to_payload_channel(patch_get): def test_args_to_payload_channel_invalid(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) with pytest.raises(ValueError) as excinfo: casda._args_to_payload(channel='one') @@ -535,7 +508,7 @@ def test_args_to_payload_channel_invalid(patch_get): def test_args_to_payload_coordinates(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) cutout_coords = SkyCoord(ra=345.245*u.degree, dec=-32.125*u.degree, frame='icrs') payload = casda._args_to_payload(coordinates=cutout_coords) @@ -574,7 +547,7 @@ def test_args_to_payload_coordinates(patch_get): def test_args_to_payload_combined(patch_get): casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) cutout_coords = SkyCoord(ra=187.5*u.degree, dec=-60.0*u.degree, frame='icrs') payload = casda._args_to_payload(coordinates=cutout_coords, channel=(17, 23)) assert payload['POS'].startswith('CIRCLE 187') @@ -593,7 +566,7 @@ def test_download_file(patch_get): 'http://casda.csiro.au/download/web/111-000-111-000/askap_img.fits.checksum', 'https://ingest.pawsey.org.au/casda-prd-as110-01/dc52217/primary_images/RACS-DR1_0000%2B18A.fits?security=stuff'] casda = Casda() - casda.login(username=USERNAME, password=PASSWORD) + fake_login(casda, USERNAME, PASSWORD) # skip the actual downloading of the file download_mock = MagicMock() diff --git a/astroquery/casda/tests/test_casda_remote.py b/astroquery/casda/tests/test_casda_remote.py index 388cc1d9c0..3c2b20fc39 100644 --- a/astroquery/casda/tests/test_casda_remote.py +++ b/astroquery/casda/tests/test_casda_remote.py @@ -65,16 +65,15 @@ def test_stage_data(self, cached_credentials): assert str(urls[1]).endswith('image_cube_g300to310.q.fits.checksum') assert len(urls) == 2 - @pytest.mark.skipif(('CASDA_USER' not in os.environ or - 'CASDA_PASSWD' not in os.environ), + @pytest.mark.skipif(('CASDA_USER' not in os.environ), reason='Requires real CASDA user/password (CASDA_USER ' - 'and CASDA_PASSWD environment variables)') - def test_cutout(self): + 'and CASDA_PASSWD environment variables or password in keyring)') + def test_cutout(self, cached_credentials): prefix = 'https://data.csiro.au/casda_vo_proxy/vo/datalink/links?ID=' access_urls = [prefix + 'cube-44705'] table = Table([Column(data=access_urls, name='access_url')]) casda = Casda() - casda.login(username=os.environ['CASDA_USER'], password=os.environ['CASDA_PASSWD']) + casda.login(username=os.environ['CASDA_USER']) casda.POLL_INTERVAL = 3 pos = SkyCoord(196.49583333*u.deg, -62.7*u.deg) urls = casda.cutout(table, coordinates=pos, radius=15*u.arcmin) From 95b41d1ff98e6bbc9ff4290a84e019556f3b2ac4 Mon Sep 17 00:00:00 2001 From: James Dempsey Date: Thu, 5 May 2022 08:59:46 +1000 Subject: [PATCH 5/6] Update exception tests to use match --- astroquery/casda/tests/test_casda.py | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/astroquery/casda/tests/test_casda.py b/astroquery/casda/tests/test_casda.py index de09b8aa16..80be64f286 100644 --- a/astroquery/casda/tests/test_casda.py +++ b/astroquery/casda/tests/test_casda.py @@ -136,11 +136,9 @@ def test_login_no_default_user(): assert casda._authenticated is False assert casda.USERNAME == '' - with pytest.raises(LoginError) as excinfo: + with pytest.raises(LoginError, match=r"If you do not pass a username to login\(\),") as excinfo: Casda.login() - assert "If you do not pass a username to login()," in str(excinfo.value) - assert casda._authenticated is False assert casda.USERNAME == '' assert hasattr(casda, '_auth') is False @@ -286,11 +284,9 @@ def test_filter_out_unreleased(): def test_stage_data_unauthorised(patch_get): table = Table() - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, match=r"Credentials must be supplied") as excinfo: Casda.stage_data(table) - assert "Credentials must be supplied" in str(excinfo.value) - def test_stage_data_empty(patch_get): table = Table() @@ -323,11 +319,9 @@ def test_stage_data_no_link(patch_get): fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, match=r"You do not have access to any of the requested data files\.") as excinfo: casda.stage_data(table) - assert "You do not have access to any of the requested data files." in str(excinfo.value) - def test_stage_data(patch_get): prefix = 'https://somewhere/casda/datalink/links?' @@ -370,9 +364,9 @@ def test_cutout_no_args(patch_get): casda = Casda() fake_login(casda, USERNAME, PASSWORD) casda.POLL_INTERVAL = 1 - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, + match=r"Please provide cutout parameters such as coordinates, band or channel\.") as excinfo: casda.cutout(table) - assert "Please provide cutout parameters such as coordinates, band or channel." in str(excinfo.value) def test_cutout_unauthorised(patch_get): @@ -384,9 +378,8 @@ def test_cutout_unauthorised(patch_get): radius = 30*u.arcmin centre = SkyCoord(ra, dec) - with pytest.raises(ValueError) as excinfo: + with pytest.raises(ValueError, match=r"Credentials must be supplied to download CASDA image data") as excinfo: Casda.cutout(table, coordinates=centre, radius=radius, verbose=True) - assert "Credentials must be supplied to download CASDA image data" in str(excinfo.value) def test_cutout_no_table(patch_get): From 740b05acc45408cddff7f987e0c3b5a23ca2c2eb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brigitta=20Sip=C5=91cz?= Date: Thu, 5 May 2022 17:38:29 -0700 Subject: [PATCH 6/6] Adding jd-au to mailmap --- .mailmap | 1 + 1 file changed, 1 insertion(+) diff --git a/.mailmap b/.mailmap index 9af20e3937..d6d3dd8d98 100644 --- a/.mailmap +++ b/.mailmap @@ -28,6 +28,7 @@ Hans Moritz Guenter Henrik Norman Henrik Norman Jaladh Singhal +James Dempsey Javier Ballester Javier Duran Javier Duran