From cd1af23c5f1f3915441235894d52b1f22bd972ab Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Fri, 19 Nov 2021 12:16:58 -0800 Subject: [PATCH 1/9] Fixed alma integration tests --- astroquery/alma/__init__.py | 7 - astroquery/alma/core.py | 11 +- astroquery/alma/tests/test_alma_remote.py | 165 ++++++++-------------- setup.cfg | 2 +- 4 files changed, 62 insertions(+), 123 deletions(-) diff --git a/astroquery/alma/__init__.py b/astroquery/alma/__init__.py index 00daa75e33..cdf82af51d 100644 --- a/astroquery/alma/__init__.py +++ b/astroquery/alma/__init__.py @@ -11,8 +11,6 @@ 'https://almascience.nrao.edu', 'https://almascience.nao.ac.jp'] -_test_url_list = ['https://beta.cadc-ccda.hia-ha.nrc-cnrc.gc.ca'] - auth_urls = ['asa.alma.cl', 'rh-cas.alma.cl'] @@ -27,11 +25,6 @@ class Conf(_config.ConfigNamespace): _url_list, 'The ALMA Archive mirror to use.') - test_archive_url = _config.ConfigItem( - _test_url_list, - 'ALMA Archive Test Mirrors (temporary)' - ) - auth_url = _config.ConfigItem( auth_urls, 'ALMA Central Authentication Service URLs' diff --git a/astroquery/alma/core.py b/astroquery/alma/core.py index e0a592354e..95d5084dd3 100644 --- a/astroquery/alma/core.py +++ b/astroquery/alma/core.py @@ -677,7 +677,7 @@ def _HEADER_data_size(self, files): return data_sizes, totalsize.to(u.GB) def download_files(self, files, savedir=None, cache=True, - continuation=True, skip_unauthorized=True): + continuation=True, skip_unauthorized=True,): """ Given a list of file URLs, download them @@ -711,8 +711,7 @@ def download_files(self, files, savedir=None, cache=True, for fileLink in unique(files): log.debug("Downloading {0} to {1}".format(fileLink, savedir)) try: - check_filename = self._request('HEAD', fileLink, auth=auth, - stream=True) + check_filename = self._request('HEAD', fileLink, auth=auth) check_filename.raise_for_status() except requests.HTTPError as ex: if ex.response.status_code == 401: @@ -748,7 +747,7 @@ def download_files(self, files, savedir=None, cache=True, auth=auth, cache=cache, method='GET', - head_safe=True, + head_safe=False, continuation=continuation) downloaded_files.append(filename) @@ -777,7 +776,7 @@ def download_files(self, files, savedir=None, cache=True, auth=auth, cache=cache, method='GET', - head_safe=True, + head_safe=False, continuation=continuation) downloaded_files.append(filename) @@ -1107,7 +1106,7 @@ def download_and_extract_files(self, urls, delete=True, regex=r'.*\.fits$', expanded_files += [x for x in files['access_url'] if filere.match(x.split('/')[-1])] else: - tar_files.append(tar_file) + tar_files.append(url) try: # get the tar files diff --git a/astroquery/alma/tests/test_alma_remote.py b/astroquery/alma/tests/test_alma_remote.py index 3897a24f76..79ff8ea1f7 100644 --- a/astroquery/alma/tests/test_alma_remote.py +++ b/astroquery/alma/tests/test_alma_remote.py @@ -7,18 +7,17 @@ import os from urllib.parse import urlparse import re -from unittest.mock import Mock, patch +from unittest.mock import Mock, MagicMock, patch from astropy import coordinates from astropy import units as u from astroquery.utils.commons import ASTROPY_LT_4_1 from .. import Alma -from .. import _url_list, _test_url_list # ALMA tests involving staging take too long, leading to travis timeouts # TODO: make this a configuration item -SKIP_SLOW = True +SKIP_SLOW = False all_colnames = {'Project code', 'Source name', 'RA', 'Dec', 'Band', 'Frequency resolution', 'Integration', 'Release date', @@ -29,27 +28,17 @@ 'QA2 Status', 'Group ous id', 'Pub'} -def get_client(): +@pytest.fixture +def alma(request): alma = Alma() - # need this to point alma to a different test site - # alma package __init__.py mentions test sites but I don't know how the - # mechanism is supposed to be used - from .. import core - core.ALMA_TAP_PATH = 'obscore' - alma.archive_url = 'https://alma.cadc-ccda.hia-iha.nrc-cnrc.gc.ca/' + alma_site = request.config.getoption('--alma-site', + 'almascience.org') + alma.archive_url = 'https://{}'.format(alma_site) return alma @pytest.mark.remote_data class TestAlma: - def setup_class(cls): - pass - # new test server - # this server seems not to serve a help page? - # Alma.archive_url = "https://2016-03.asa-test.alma.cl/aq/" - # starting somewhere between Nov 2015 and Jan 2016, the beta server - # stopped serving the actual data, making all staging attempts break - @pytest.fixture() def temp_dir(self, request): my_temp_dir = tempfile.mkdtemp() @@ -59,8 +48,8 @@ def fin(): request.addfinalizer(fin) return my_temp_dir - def test_public(self): - alma = get_client() + def test_public(self, alma): + assert alma.archive_url == 'https://almascience.eso.org' results = alma.query(payload=None, public=True, maxrec=100) assert len(results) == 100 for row in results: @@ -70,8 +59,7 @@ def test_public(self): for row in results: assert row['data_rights'] == 'Proprietary' - def test_SgrAstar(self, temp_dir): - alma = get_client() + def test_SgrAstar(self, temp_dir, alma): alma.cache_location = temp_dir result_s = alma.query_object('Sgr A*', legacy_columns=True) @@ -80,17 +68,14 @@ def test_SgrAstar(self, temp_dir): # "The Brick", g0.253, is in this one # assert b'2011.0.00217.S' in result_c['Project code'] # missing cycle 1 data - def test_docs_example(self, temp_dir): - alma = get_client() + def test_docs_example(self, temp_dir, alma): alma.cache_location = temp_dir rslt = alma.query(payload=dict(obs_creator_name='*Ginsburg*')) assert 'ADS/JAO.ALMA#2013.1.00269.S' in rslt['obs_publisher_did'] - def test_freq(self): - alma = get_client() - + def test_freq(self, alma): payload = {'frequency': '85..86'} result = alma.query(payload) assert len(result) > 0 @@ -103,18 +88,16 @@ def test_freq(self): @pytest.mark.skipif("SKIP_SLOW", reason="Extremely slow due to limitations of " "the implementation") - def test_bands(self): - alma = get_client() + def test_bands(self, alma): payload = {'band_list': ['5', '7']} result = alma.query(payload) assert len(result) > 0 for row in result: assert ('5' in row['band_list']) or ('7' in row['band_list']) - def test_equivalent_columns(self): + def test_equivalent_columns(self, alma): # this test is to ensure that queries using original column names # return the same results as the ones that use ObsCore names - alma = get_client() # original result_orig = alma.query(payload={'project_code': '2011.0.00131.S'}, legacy_columns=True) @@ -126,8 +109,7 @@ def test_equivalent_columns(self): for row in result_obscore: assert row['Project code'] == '2011.0.00131.S' - def test_alma_source_name(self): - alma = get_client() + def test_alma_source_name(self, alma): payload = {'source_name_alma': 'GRB021004'} result = alma.query(payload) assert len(result) > 0 @@ -135,15 +117,13 @@ def test_alma_source_name(self): assert 'GRB021004' == row['target_name'] @pytest.mark.skipif("SKIP_SLOW", reason="Known issue") - def test_ra_dec(self): - alma = get_client() + def test_ra_dec(self, alma): payload = {'ra_dec': '181.0192d -0.01928d'} result = alma.query(payload) assert len(result) > 0 @pytest.mark.skipif("SKIP_SLOW") - def test_m83(self, temp_dir, recwarn): - alma = get_client() + def test_m83(self, temp_dir, alma): alma.cache_location = temp_dir m83_data = alma.query_object('M83', science=True, legacy_columns=True) @@ -170,8 +150,7 @@ def test_m83(self, temp_dir, recwarn): # ' otherwise you may need to create a fresh astroquery.Alma instance.')) @pytest.mark.skipif("SKIP_SLOW", reason="Known issue") - def test_stage_data(self, temp_dir, recwarn): - alma = get_client() + def test_stage_data(self, temp_dir, alma): alma.cache_location = temp_dir result_s = alma.query_object('Sgr A*', legacy_columns=True) @@ -201,11 +180,10 @@ def test_stage_data(self, temp_dir, recwarn): break assert found, 'URL to uid___A002_X40d164_X1b3 expected' - def test_stage_data_listall(self, temp_dir, recwarn): + def test_stage_data_listall(self, temp_dir, alma): """ test for expanded capability created in #1683 """ - alma = get_client() alma.cache_location = temp_dir uid = 'uid://A001/X12a3/Xe9' @@ -233,11 +211,10 @@ def test_stage_data_listall(self, temp_dir, recwarn): assert res['mous_uid'] == uid assert len(result2) > len(result1) - def test_stage_data_json(self, temp_dir, recwarn): + def test_stage_data_json(self, temp_dir, alma): """ test for json returns """ - alma = get_client() alma.cache_location = temp_dir uid = 'uid://A001/X12a3/Xe9' @@ -249,9 +226,8 @@ def test_stage_data_json(self, temp_dir, recwarn): # this no longer works alma.stage_data(uid, return_json=True) - def test_data_proprietary(self): + def test_data_proprietary(self, alma): # public - alma = get_client() assert not alma.is_proprietary('uid://A001/X12a3/Xe9') IVOA_DATE_FORMAT = "%Y-%m-%dT%H:%M:%S.%f" now = datetime.utcnow().strftime(IVOA_DATE_FORMAT)[:-3] @@ -265,8 +241,7 @@ def test_data_proprietary(self): with pytest.raises(AttributeError): alma.is_proprietary('uid://NON/EXI/STING') - def test_data_info(self, temp_dir): - alma = get_client() + def test_data_info(self, temp_dir, alma): alma.cache_location = temp_dir uid = 'uid://A001/X12a3/Xe9' @@ -304,45 +279,37 @@ def test_data_info(self, temp_dir): 'access_url'] assert comparison.all() - def test_download_data(self, temp_dir): + def test_download_data(self, temp_dir, alma): # test only fits files from a program - def myrequests(op, file_url, **kwargs): - # this is to avoid downloading the actual files - if op == 'HEAD': - return Mock(headers={'Content-Type': 'fits'}) - else: - return file_url.split('/')[-1] - alma = get_client() alma.cache_location = temp_dir uid = 'uid://A001/X12a3/Xe9' data_info = alma.get_data_info(uid, expand_tarfiles=True) fitsre = re.compile(r'.*\.fits$') - alma._request = Mock(side_effect=myrequests) + # skip the actual downloading of the file + download_mock = MagicMock() + # following line require to make alma picklable + download_mock.__reduce__ = lambda self: (MagicMock, ()) + alma._download_file = download_mock urls = [x['access_url'] for x in data_info if fitsre.match(x['access_url'])] results = alma.download_files(urls, temp_dir) - alma._request.assert_called() + alma._download_file.call_count == len(results) assert len(results) == len(urls) - # each url triggers 2 calls: HEAD and GET - assert len(urls)*2 == len(alma._request.mock_calls) - - def test_download_and_extract(self, temp_dir): - def myrequests(op, file_url, **kwargs): - # this is to avoid downloading the actual files - if op == 'HEAD': - return Mock(headers={'Content-Type': 'fits'}) - else: - return file_url.split('/')[-1] - alma = get_client() + + def test_download_and_extract(self, temp_dir, alma): alma.cache_location = temp_dir - alma._request = Mock(side_effect=myrequests) alma._cycle0_tarfile_content_table = {'ID': ''} uid = 'uid://A001/X12a3/Xe9' data_info = alma.get_data_info(uid, expand_tarfiles=False) aux_tar_file = [x for x in data_info['access_url'] if 'auxiliary' in x] assert 1 == len(aux_tar_file) + download_mock = MagicMock() + # following line require to make alma picklable + download_mock.__reduce__ = lambda self: (MagicMock, ()) + alma._download_file = download_mock + # there are no FITS files in the auxiliary file assert not alma.download_and_extract_files(aux_tar_file) @@ -350,7 +317,7 @@ def myrequests(op, file_url, **kwargs): downloaded = alma.download_and_extract_files(aux_tar_file, regex=r'.*\.py') assert len(downloaded) > 1 - assert len(downloaded)*2 == len(alma._request.mock_calls) + assert download_mock.call_count == len(downloaded) # ASDM files cannot be expanded. asdm_url = [x for x in data_info['access_url'] if 'asdm' in x][0] @@ -369,15 +336,13 @@ def myrequests(op, file_url, **kwargs): with patch('astroquery.alma.core.os.remove') as delete_mock: downloaded_asdm = alma.download_and_extract_files( [asdm_url], include_asdm=True, regex=r'.*\.py') - delete_mock.assert_called_once_with(asdm_url.split('/')[-1]) + delete_mock.assert_called_once_with( + 'cache_path/' + asdm_url.split('/')[-1]) assert downloaded_asdm == [os.path.join(temp_dir, 'foo.py')] @pytest.mark.skipif("SKIP_SLOW", reason="Known issue") - def test_doc_example(self, temp_dir): - alma = get_client() + def test_doc_example(self, temp_dir, alma): alma.cache_location = temp_dir - alma2 = get_client() - alma2.cache_location = temp_dir m83_data = alma.query_object('M83', legacy_columns=True) # the order can apparently sometimes change # These column names change too often to keep testing. @@ -404,15 +369,14 @@ def test_doc_example(self, temp_dir): totalsize_mous1 = mous1['size'].sum() * u.Unit(mous1['size'].unit) assert (totalsize_mous1.to(u.B) > 1.9*u.GB) - mous = alma2.stage_data('uid://A002/X3216af/X31') + mous = alma.stage_data('uid://A002/X3216af/X31') totalsize_mous = mous['size'].sum() * u.Unit(mous['size'].unit) # More recent ALMA request responses do not include any information # about file size, so we have to allow for the possibility that all # file sizes are replaced with -1 assert (totalsize_mous.to(u.GB).value > 52) - def test_query(self, temp_dir): - alma = get_client() + def test_query(self, temp_dir, alma): alma.cache_location = temp_dir result = alma.query(payload={'start_date': '<11-11-2011'}, @@ -434,9 +398,8 @@ def test_query(self, temp_dir): # assert len(result) == 1 @pytest.mark.skipif("SKIP_SLOW", reason="ra dec search known issue") - def test_misc(self): + def test_misc(self, alma): # miscellaneous set of common tests - alma = get_client() # # alma.query_region(coordinate=orionkl_coords, radius=4 * u.arcmin, # public=False, science=False) @@ -498,9 +461,8 @@ def test_misc(self): assert 'ginsburg' in row['obs_creator_name'].lower() @pytest.mark.skipif("SKIP_SLOW") - def test_user(self): + def test_user(self, alma): # miscellaneous set of tests from current users - alma = get_client() rslt = alma.query({'band_list': [6], 'project_code': '2012.1.*'}, legacy_columns=True) for row in rslt: @@ -512,9 +474,8 @@ def test_user(self): @pytest.mark.xfail @pytest.mark.bigdata @pytest.mark.skipif("SKIP_SLOW") - def test_cycle1(self, temp_dir): + def test_cycle1(self, temp_dir, alma): # About 500 MB - alma = get_client() alma.cache_location = temp_dir target = 'NGC4945' project_code = '2012.1.00912.S' @@ -560,10 +521,8 @@ def test_cycle1(self, temp_dir): @pytest.mark.skipif("SKIP_SLOW") @pytest.mark.skip("Not working anymore") - def test_cycle0(self, temp_dir): + def test_cycle0(self, temp_dir, alma): # About 20 MB - - alma = get_client() alma.cache_location = temp_dir target = 'NGC4945' @@ -597,9 +556,7 @@ def test_cycle0(self, temp_dir): # There are 10 small files, but only 8 unique assert len(data) == 8 - def test_keywords(self, temp_dir): - - alma = get_client() + def test_keywords(self, temp_dir, alma): alma.help_tap() result = alma.query_tap( @@ -613,8 +570,7 @@ def test_keywords(self, temp_dir): @pytest.mark.remote_data -def test_project_metadata(): - alma = get_client() +def test_project_metadata(alma): metadata = alma.get_project_metadata('2013.1.00269.S') assert metadata == ['Sgr B2, a high-mass molecular cloud in our Galaxy\'s ' 'Central Molecular Zone, is the most extreme site of ' @@ -643,11 +599,9 @@ def test_project_metadata(): @pytest.mark.remote_data -@pytest.mark.parametrize('dataarchive_url', _test_url_list) @pytest.mark.skip('Not working for now - Investigating') -def test_staging_postfeb2020(dataarchive_url): +def test_staging_postfeb2020(alma): - alma = get_client() tbl = alma.stage_data('uid://A001/X121/X4ba') assert 'mous_uid' in tbl.colnames @@ -656,11 +610,8 @@ def test_staging_postfeb2020(dataarchive_url): @pytest.mark.remote_data -@pytest.mark.parametrize('dataarchive_url', _url_list) @pytest.mark.skip('Not working for now - Investigating') -def test_staging_uptofeb2020(dataarchive_url): - - alma = get_client() +def test_staging_uptofeb2020(alma): tbl = alma.stage_data('uid://A001/X121/X4ba') assert 'mous_uid' in tbl.colnames @@ -671,29 +622,25 @@ def test_staging_uptofeb2020(dataarchive_url): @pytest.mark.remote_data -@pytest.mark.parametrize('dataarchive_url', _test_url_list) -def test_staging_stacking(dataarchive_url): - alma = get_client() - +def test_staging_stacking(alma): alma.stage_data(['uid://A001/X13d5/X1d', 'uid://A002/X3216af/X31', 'uid://A001/X12a3/X240']) @pytest.mark.remote_data -@pytest.mark.skipif("SKIP_SLOW", "Huge data file download") -@pytest.mark.parametrize('dataarchive_url', _test_url_list) -def test_big_download_regression(dataarchive_url): +@pytest.mark.skipif("SKIP_SLOW", reason="Huge data file download") +def test_big_download_regression(alma): """ Regression test for #2020/#2021 - this download fails if logging tries to load the whole data file into memory. """ - result = Alma.query({'project_code': '2013.1.01365.S'}) + result = alma.query({'project_code': '2013.1.01365.S'}) uids = np.unique(result['member_ous_uid']) - files = Alma.get_data_info(uids) + files = alma.get_data_info(uids) # we may need to change the cache dir for this to work on testing machines? # savedir='/big/data/path/' # Alma.cache_dir=savedir # this is a big one that fails - Alma.download_files([files['access_url'][3]]) + alma.download_files([files['access_url'][3]]) diff --git a/setup.cfg b/setup.cfg index 71f8016780..2266bc67a2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -32,7 +32,7 @@ astropy_header = true text_file_format = rst xfail_strict = true remote_data_strict = true -addopts = --doctest-rst +#addopts = --doctest-rst filterwarnings = error ignore: Experimental:UserWarning: From 4a16508ac564ddb9c6423144c194fca971f3ef3e Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Fri, 19 Nov 2021 22:51:52 -0800 Subject: [PATCH 2/9] Rework pytest options --- astroquery/alma/tests/test_alma_remote.py | 2 +- astroquery/conftest.py | 11 +++++++++++ setup.cfg | 2 +- 3 files changed, 13 insertions(+), 2 deletions(-) diff --git a/astroquery/alma/tests/test_alma_remote.py b/astroquery/alma/tests/test_alma_remote.py index 79ff8ea1f7..14a28dc3eb 100644 --- a/astroquery/alma/tests/test_alma_remote.py +++ b/astroquery/alma/tests/test_alma_remote.py @@ -17,7 +17,7 @@ # ALMA tests involving staging take too long, leading to travis timeouts # TODO: make this a configuration item -SKIP_SLOW = False +SKIP_SLOW = True all_colnames = {'Project code', 'Source name', 'RA', 'Dec', 'Band', 'Frequency resolution', 'Integration', 'Release date', diff --git a/astroquery/conftest.py b/astroquery/conftest.py index 2a76795d23..8b6aeca802 100644 --- a/astroquery/conftest.py +++ b/astroquery/conftest.py @@ -47,3 +47,14 @@ def pytest_configure(config): packagename = os.path.basename(os.path.dirname(__file__)) TESTED_VERSIONS[packagename] = version TESTED_VERSIONS['astropy_helpers'] = astropy_helpers_version + +def pytest_addoption(parser): + parser.addoption( + '--alma-site', + action='store', + default='almascience.org', + help='ALMA site', + choices={'almascience.nrao.edu', + 'almascience.eso.org', + 'almascience.nao.ac.jp'} + ) diff --git a/setup.cfg b/setup.cfg index 2266bc67a2..71f8016780 100644 --- a/setup.cfg +++ b/setup.cfg @@ -32,7 +32,7 @@ astropy_header = true text_file_format = rst xfail_strict = true remote_data_strict = true -#addopts = --doctest-rst +addopts = --doctest-rst filterwarnings = error ignore: Experimental:UserWarning: From 905c55758254e07723e4947649f3f3814bb79c43 Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Sun, 21 Nov 2021 20:46:26 -0800 Subject: [PATCH 3/9] Changed the scope of --alma-site pytest argument --- astroquery/alma/core.py | 2 +- astroquery/alma/tests/test_alma_remote.py | 6 ++++++ astroquery/conftest.py | 7 +++---- docs/alma/alma.rst | 4 ++-- 4 files changed, 12 insertions(+), 7 deletions(-) diff --git a/astroquery/alma/core.py b/astroquery/alma/core.py index 95d5084dd3..337daee698 100644 --- a/astroquery/alma/core.py +++ b/astroquery/alma/core.py @@ -110,7 +110,7 @@ }, 'Energy': { 'Frequency (GHz)': ['frequency', 'frequency', _gen_numeric_sql], - 'Bandwidth (GHz)': ['bandwidth', 'bandwidth', _gen_numeric_sql], + 'Bandwidth (Hz)': ['bandwidth', 'bandwidth', _gen_numeric_sql], 'Spectral resolution (KHz)': ['spectral_resolution', 'em_resolution', _gen_spec_res_sql], 'Band': ['band_list', 'band_list', _gen_band_list_sql] diff --git a/astroquery/alma/tests/test_alma_remote.py b/astroquery/alma/tests/test_alma_remote.py index 14a28dc3eb..5f77ab0a00 100644 --- a/astroquery/alma/tests/test_alma_remote.py +++ b/astroquery/alma/tests/test_alma_remote.py @@ -30,6 +30,12 @@ @pytest.fixture def alma(request): + """ + Returns an alma client class. `--alma-site` pytest option can be used + to have the client run against a specific site + :param request: pytest request fixture + :return: alma client to use in tests + """ alma = Alma() alma_site = request.config.getoption('--alma-site', 'almascience.org') diff --git a/astroquery/conftest.py b/astroquery/conftest.py index 8b6aeca802..6e2cdda5d4 100644 --- a/astroquery/conftest.py +++ b/astroquery/conftest.py @@ -48,13 +48,12 @@ def pytest_configure(config): TESTED_VERSIONS[packagename] = version TESTED_VERSIONS['astropy_helpers'] = astropy_helpers_version + def pytest_addoption(parser): parser.addoption( '--alma-site', action='store', default='almascience.org', - help='ALMA site', - choices={'almascience.nrao.edu', - 'almascience.eso.org', - 'almascience.nao.ac.jp'} + help='ALMA site (almascience.nrao.edu, almascience.eso.org or ' + 'almascience.nao.ac.jp for example)' ) diff --git a/docs/alma/alma.rst b/docs/alma/alma.rst index 647ae1f2fb..fa1d883535 100644 --- a/docs/alma/alma.rst +++ b/docs/alma/alma.rst @@ -47,7 +47,7 @@ You can get interactive help to find out what keywords to query for: Energy Frequency (GHz) frequency frequency - Bandwidth (GHz) bandwidth bandwidth + Bandwidth (Hz) bandwidth bandwidth Spectral resolution (KHz) spectral_resolution em_resolution Band band_list band_list @@ -210,7 +210,7 @@ their types. asdm_uid char(32*) UID of the ASDM containing this Field. authors char(4000*) Full list of first author and all co-authors band_list char(30*) Space delimited list of bands - bandwidth double GHz Total Bandwidth + bandwidth double Hz Total Bandwidth bib_reference char(30*) Bibliography code calib_level int calibration level (2 or 3). 2 if product_type = MOUS, 3 if product_type = GOUS cont_sensitivity_bandwidth double mJy/beam Estimated noise in the aggregated continuum bandwidth. Note this is an indication only, it does not include the effects of flagging or dynamic range limitations. From 883bd607bbd0cd3d04d1e255ff2bd724064a802e Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Sun, 21 Nov 2021 21:35:45 -0800 Subject: [PATCH 4/9] Bumped coverage --- astroquery/alma/core.py | 28 +++++++++++------------ astroquery/alma/tests/test_alma.py | 17 ++++++++++++++ astroquery/alma/tests/test_alma_remote.py | 2 +- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/astroquery/alma/core.py b/astroquery/alma/core.py index 337daee698..92d7789e37 100644 --- a/astroquery/alma/core.py +++ b/astroquery/alma/core.py @@ -708,21 +708,21 @@ def download_files(self, files, savedir=None, cache=True, downloaded_files = [] if savedir is None: savedir = self.cache_location - for fileLink in unique(files): - log.debug("Downloading {0} to {1}".format(fileLink, savedir)) + for file_link in unique(files): + log.debug("Downloading {0} to {1}".format(file_link, savedir)) try: - check_filename = self._request('HEAD', fileLink, auth=auth) + check_filename = self._request('HEAD', file_link, auth=auth) check_filename.raise_for_status() except requests.HTTPError as ex: if ex.response.status_code == 401: if skip_unauthorized: log.info("Access denied to {url}. Skipping to" - " next file".format(url=fileLink)) + " next file".format(url=file_link)) continue else: raise(ex) - if 'text/html' in check_filename.headers['Content-Type']: + if 'text/html' in check_filename.headers.get('Content-Type', ''): raise ValueError("Bad query. This can happen if you " "attempt to download proprietary " "data when not logged in") @@ -731,7 +731,7 @@ def download_files(self, files, savedir=None, cache=True, filename = re.search("filename=(.*)", check_filename.headers['Content-Disposition']).groups()[0] except KeyError: - log.info(f"Unable to find filename for {fileLink} " + log.info(f"Unable to find filename for {file_link} " "(missing Content-Disposition in header). " "Skipping to next file.") continue @@ -741,7 +741,7 @@ def download_files(self, files, savedir=None, cache=True, filename) try: - self._download_file(fileLink, + self._download_file(file_link, filename, timeout=self.TIMEOUT, auth=auth, @@ -755,22 +755,22 @@ def download_files(self, files, savedir=None, cache=True, if ex.response.status_code == 401: if skip_unauthorized: log.info("Access denied to {url}. Skipping to" - " next file".format(url=fileLink)) + " next file".format(url=file_link)) continue else: raise(ex) elif ex.response.status_code == 403: - log.error("Access denied to {url}".format(url=fileLink)) - if 'dataPortal' in fileLink and 'sso' not in fileLink: + log.error("Access denied to {url}".format(url=file_link)) + if 'dataPortal' in file_link and 'sso' not in file_link: log.error("The URL may be incorrect. Try using " "{0} instead of {1}" - .format(fileLink.replace('dataPortal/', - 'dataPortal/sso/'), - fileLink)) + .format(file_link.replace('dataPortal/', + 'dataPortal/sso/'), + file_link)) raise ex elif ex.response.status_code == 500: # empirically, this works the second time most of the time... - self._download_file(fileLink, + self._download_file(file_link, filename, timeout=self.TIMEOUT, auth=auth, diff --git a/astroquery/alma/tests/test_alma.py b/astroquery/alma/tests/test_alma.py index dfa9580c15..4527a8d06f 100644 --- a/astroquery/alma/tests/test_alma.py +++ b/astroquery/alma/tests/test_alma.py @@ -410,3 +410,20 @@ def test_galactic_query(): radius=1*u.deg, get_query_payload=True) assert result['ra_dec'] == SkyCoord(0*u.deg, 0*u.deg, frame='galactic').icrs.to_string() + ", 1.0" + + +def test_download_files(): + def _requests_mock(method, url, **kwargs): + response = Mock() + response.headers = { + 'Content-Disposition': 'attachment; ' + 'filename="{}"'.format(url.split('/')[-1])} + return response + + def _download_file_mock(url, file_name, **kwargs): + return file_name + alma = Alma() + alma._request = Mock(side_effect=_requests_mock) + alma._download_file = Mock(side_effect=_download_file_mock) + downloaded_files = alma.download_files(['https://location/file1']) + assert len(downloaded_files) == 1 diff --git a/astroquery/alma/tests/test_alma_remote.py b/astroquery/alma/tests/test_alma_remote.py index 5f77ab0a00..2894ab297d 100644 --- a/astroquery/alma/tests/test_alma_remote.py +++ b/astroquery/alma/tests/test_alma_remote.py @@ -312,7 +312,7 @@ def test_download_and_extract(self, temp_dir, alma): aux_tar_file = [x for x in data_info['access_url'] if 'auxiliary' in x] assert 1 == len(aux_tar_file) download_mock = MagicMock() - # following line require to make alma picklable + # following line is required to make alma picklable download_mock.__reduce__ = lambda self: (MagicMock, ()) alma._download_file = download_mock From 88be2e9ea8034a4213827843d600353ed68611d7 Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Sun, 21 Nov 2021 22:02:26 -0800 Subject: [PATCH 5/9] Bumped coverage --- astroquery/alma/tests/test_alma.py | 16 +++++++++++++++- setup.cfg | 2 +- 2 files changed, 16 insertions(+), 2 deletions(-) diff --git a/astroquery/alma/tests/test_alma.py b/astroquery/alma/tests/test_alma.py index 4527a8d06f..9c56abc23d 100644 --- a/astroquery/alma/tests/test_alma.py +++ b/astroquery/alma/tests/test_alma.py @@ -417,7 +417,7 @@ def _requests_mock(method, url, **kwargs): response = Mock() response.headers = { 'Content-Disposition': 'attachment; ' - 'filename="{}"'.format(url.split('/')[-1])} + 'filename={}'.format(url.split('/')[-1])} return response def _download_file_mock(url, file_name, **kwargs): @@ -427,3 +427,17 @@ def _download_file_mock(url, file_name, **kwargs): alma._download_file = Mock(side_effect=_download_file_mock) downloaded_files = alma.download_files(['https://location/file1']) assert len(downloaded_files) == 1 + assert downloaded_files[0].split('/')[-1] == 'file1' + + alma._request.reset_mock() + alma._download_file.reset_mock() + downloaded_files = alma.download_files(['https://location/file1', + 'https://location/file2']) + assert len(downloaded_files) == 2 + + # error cases + alma._request = Mock() + # no Content-Disposition results in no downloaded file + alma._request.return_value = Mock(headers={}) + result = alma.download_files(['https://location/file1']) + assert not result diff --git a/setup.cfg b/setup.cfg index 71f8016780..2266bc67a2 100644 --- a/setup.cfg +++ b/setup.cfg @@ -32,7 +32,7 @@ astropy_header = true text_file_format = rst xfail_strict = true remote_data_strict = true -addopts = --doctest-rst +#addopts = --doctest-rst filterwarnings = error ignore: Experimental:UserWarning: From 6749cc1461b3808c940297e1f3d508240c02f582 Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Mon, 22 Nov 2021 11:01:08 -0800 Subject: [PATCH 6/9] Bumped coverage --- astroquery/alma/tests/test_alma_remote.py | 1 - 1 file changed, 1 deletion(-) diff --git a/astroquery/alma/tests/test_alma_remote.py b/astroquery/alma/tests/test_alma_remote.py index 2894ab297d..8ecf44c2b5 100644 --- a/astroquery/alma/tests/test_alma_remote.py +++ b/astroquery/alma/tests/test_alma_remote.py @@ -55,7 +55,6 @@ def fin(): return my_temp_dir def test_public(self, alma): - assert alma.archive_url == 'https://almascience.eso.org' results = alma.query(payload=None, public=True, maxrec=100) assert len(results) == 100 for row in results: From 4adda13b4f77b8707a82ae6900e55239c657b67c Mon Sep 17 00:00:00 2001 From: Adrian Damian Date: Mon, 22 Nov 2021 11:07:33 -0800 Subject: [PATCH 7/9] Fixed test --- astroquery/alma/tests/test_alma.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/astroquery/alma/tests/test_alma.py b/astroquery/alma/tests/test_alma.py index 9c56abc23d..5198230a22 100644 --- a/astroquery/alma/tests/test_alma.py +++ b/astroquery/alma/tests/test_alma.py @@ -427,7 +427,7 @@ def _download_file_mock(url, file_name, **kwargs): alma._download_file = Mock(side_effect=_download_file_mock) downloaded_files = alma.download_files(['https://location/file1']) assert len(downloaded_files) == 1 - assert downloaded_files[0].split('/')[-1] == 'file1' + assert downloaded_files[0].endswith('file1') alma._request.reset_mock() alma._download_file.reset_mock() From b2529b0c9b6c0ca11efb877bc44110862970ab52 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brigitta=20Sip=C5=91cz?= Date: Mon, 22 Nov 2021 18:31:55 -0800 Subject: [PATCH 8/9] Doctests should run for narrative docs --- setup.cfg | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/setup.cfg b/setup.cfg index 2266bc67a2..71f8016780 100644 --- a/setup.cfg +++ b/setup.cfg @@ -32,7 +32,7 @@ astropy_header = true text_file_format = rst xfail_strict = true remote_data_strict = true -#addopts = --doctest-rst +addopts = --doctest-rst filterwarnings = error ignore: Experimental:UserWarning: From b2bd97cf40b252d6687df6decb0ac4a7a7db21a4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Brigitta=20Sip=C5=91cz?= Date: Mon, 22 Nov 2021 18:35:20 -0800 Subject: [PATCH 9/9] Adding --alma-site to changelog --- CHANGES.rst | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index fe9716f285..d844d68292 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -16,6 +16,8 @@ sdss Infrastructure, Utility and Other Changes and Additions ------------------------------------------------------- +- Adding ``--alma-site`` pytest option for testing to have a control over + which specific site to test. [#2224] 0.4.4 (2021-11-17)