From 8ea5a53cafd480531b0da04cb53c903fa4ec973b Mon Sep 17 00:00:00 2001 From: Tom Aldcroft Date: Wed, 27 Sep 2023 11:27:12 -0400 Subject: [PATCH] Add version kwarg and fix tests --- agasc/agasc.py | 33 ++++++++++++------ agasc/tests/test_agasc_1.py | 28 +++++++++++++-- agasc/tests/test_agasc_2.py | 55 ++++++++++++++++++----------- agasc/tests/test_agasc_healpix.py | 57 +++++++++++++++---------------- 4 files changed, 109 insertions(+), 64 deletions(-) diff --git a/agasc/agasc.py b/agasc/agasc.py index 81f3a62c..9bd5d64d 100644 --- a/agasc/agasc.py +++ b/agasc/agasc.py @@ -230,7 +230,11 @@ def _read_h5_table_from_open_h5_file(h5, path, row0, row1): return out -def get_agasc_filename(agasc_file: Optional[str | Path] = None, allow_rc: bool=False): +def get_agasc_filename( + agasc_file: Optional[str | Path] = None, + allow_rc: bool=False, + version: Optional[str]=None, +) -> str: """Get a matching AGASC file name from ``agasc_file``. {common_agasc_file_doc} @@ -241,6 +245,8 @@ def get_agasc_filename(agasc_file: Optional[str | Path] = None, allow_rc: bool=F AGASC file name (default=None) allow_rc : bool, optional Allow AGASC release candidate files (default=False) + version : str, optional + Version number to match (e.g. "1p8" or "1p8rc4", default=None) Returns ------- @@ -257,10 +263,10 @@ def get_agasc_filename(agasc_file: Optional[str | Path] = None, allow_rc: bool=F >>> get_agasc_filename() '/Users/aldcroft/ska/data/agasc/proseco_agasc_1p7.h5' - >>> get_agasc_filename("miniagasc") - '/Users/aldcroft/ska/data/agasc/miniagasc.h5' - >>> get_agasc_filename("proseco_agasc*") + >>> get_agasc_filename("proseco_agasc_*") '/Users/aldcroft/ska/data/agasc/proseco_agasc_1p7.h5' + >>> get_agasc_filename("proseco_agasc_*", version="1p8", allow_rc=True) + '/Users/aldcroft/ska/data/agasc/proseco_agasc_1p8rc4.h5' >>> get_agasc_filename("agas*") Traceback (most recent call last): ... @@ -292,7 +298,7 @@ def get_agasc_filename(agasc_file: Optional[str | Path] = None, allow_rc: bool=F if "AGASC_HDF5_FILE" in os.environ: return str(default_agasc_dir() / os.environ["AGASC_HDF5_FILE"]) else: - agasc_file = "proseco_agasc*" + agasc_file = "proseco_agasc_*" agasc_file = str(agasc_file) @@ -305,21 +311,26 @@ def get_agasc_filename(agasc_file: Optional[str | Path] = None, allow_rc: bool=F if not agasc_file.endswith("*"): raise ValueError("agasc_file must end with '*' or '.h5'") - agasc_file_re = agasc_file[:-1] + r"_? (1p[0-9]+) (rc[1-9][0-9]*)? \.h5$" + agasc_file_re = agasc_file[:-1] + r"(1p[0-9]+) (rc[1-9][0-9]*)? \.h5$" matches = [] for path in agasc_dir.glob("*.h5"): name = path.name if match := re.match(agasc_file_re, name, re.VERBOSE): if not allow_rc and match.group(2): continue - version = match.group(1).replace("p", ".") - rc = match.group(2) or "" - matches.append((Version(version + rc), path)) + version_str = match.group(1) + rc_str = match.group(2) or "" + if ( + version is not None + and version not in (version_str, version_str + rc_str) + ): + continue + matches.append((Version(version_str.replace("p", ".") + rc_str), path)) if len(matches) == 0: + with_version = f" with {version=}" if version is not None else "" raise FileNotFoundError( - f"No AGASC files in {agasc_dir} found matching " - f"{agasc_file}_?1p([0-9]+).h5" + f"No AGASC files in {agasc_dir}{with_version} matching {agasc_file_re}" ) # Get candidate with highest version number. Tuples are sorted lexically starting # by first element, which is the version number here. diff --git a/agasc/tests/test_agasc_1.py b/agasc/tests/test_agasc_1.py index 3782f767..b4b1be81 100644 --- a/agasc/tests/test_agasc_1.py +++ b/agasc/tests/test_agasc_1.py @@ -132,8 +132,8 @@ def test_get_agasc_filename(tmp_path, monkeypatch): for name in names: (tmp_path / name).touch() - def _check(filename, expected, allow_rc=False): - assert agasc.get_agasc_filename(filename, allow_rc) == str(expected) + def _check(filename, expected, allow_rc=False, version=None): + assert agasc.get_agasc_filename(filename, allow_rc, version) == str(expected) # Default is latest proseco_agasc in AGASC_DIR _check(None, tmp_path / "proseco_agasc_1p8.h5") @@ -150,8 +150,30 @@ def _check(filename, expected, allow_rc=False): _check("agasc*", tmp_path / "agasc1p8.h5", allow_rc=False) _check("agasc*", tmp_path / "agasc1p8.h5", allow_rc=True) + # 1p8rc2 is available but it takes the non-RC version 1p8 + _check( + "proseco_agasc_*", + tmp_path / "proseco_agasc_1p8.h5", + allow_rc=True, + version="1p8", + ) + # You can choose the RC version explicitly + _check( + "proseco_agasc_*", + tmp_path / "proseco_agasc_1p8rc2.h5", + allow_rc=True, + version="1p8rc2", + ) + # For version="1p9" only the 1p9rc1 version is available + _check( + "proseco_agasc_*", + tmp_path / "proseco_agasc_1p9rc1.h5", + allow_rc=True, + version="1p9", + ) + # Wildcard finds the right file (and double-digit version is OK) - _check("miniagasc*", tmp_path / "miniagasc_1p10.h5") + _check("miniagasc_*", tmp_path / "miniagasc_1p10.h5") # With .h5 supplied just return the file, again don't require existence. _check("agasc1p7.h5", "agasc1p7.h5") diff --git a/agasc/tests/test_agasc_2.py b/agasc/tests/test_agasc_2.py index 68373ff8..a44b667a 100644 --- a/agasc/tests/test_agasc_2.py +++ b/agasc/tests/test_agasc_2.py @@ -110,14 +110,20 @@ TEST_RADIUS = 0.6 # standard testing radius TEST_DIR = Path(__file__).parent -DATA_DIR = Path(os.environ['SKA'], 'data', 'agasc') -AGASC_FILE = {} -AGASC_FILE['1p6'] = DATA_DIR / 'miniagasc_1p6.h5' -AGASC_FILE['1p7'] = DATA_DIR / 'miniagasc.h5' # Latest release # Whether to test DS AGASC vs. agasc package HDF5 files -TEST_ASCDS = DS_AGASC_VERSION is not None and AGASC_FILE[DS_AGASC_VERSION].exists() +if DS_AGASC_VERSION is None: + TEST_ASCDS = False +else: + try: + agasc.get_agasc_filename('miniagasc_*', version=DS_AGASC_VERSION) + except FileNotFoundError: + TEST_ASCDS = False + else: + TEST_ASCDS = True +# Latest full release of miniagasc +MINIAGASC = agasc.get_agasc_filename('miniagasc_*') def get_ds_agasc_cone(ra, dec): cmd = 'mp_get_agasc -r {!r} -d {!r} -w {!r}'.format(ra, dec, TEST_RADIUS) @@ -163,9 +169,14 @@ def test_agasc_conesearch(ra, dec, version): ref_stars = get_reference_agasc_values(ra, dec, version=version) except FileNotFoundError: if os.environ.get('WRITE_AGASC_TEST_FILES'): - ref_stars = agasc.get_agasc_cone(ra, dec, radius=TEST_RADIUS, - agasc_file=AGASC_FILE[version], - date='2000:001', fix_color1=False) + ref_stars = agasc.get_agasc_cone( + ra, + dec, + radius=TEST_RADIUS, + agasc_file=agasc.get_agasc_filename('miniagasc_*', version=version), + date='2000:001', + fix_color1=False + ) test_file = get_test_file(ra, dec, version) print(f'\nWriting {test_file} based on miniagasc\n') ref_stars.write(test_file, format='fits') @@ -186,8 +197,9 @@ def test_against_ds_agasc(ra, dec): def _test_agasc(ra, dec, ref_stars, version='1p7'): + agasc_file = agasc.get_agasc_filename('miniagasc_*', version=version) stars1 = agasc.get_agasc_cone(ra, dec, radius=TEST_RADIUS, - agasc_file=AGASC_FILE[version], + agasc_file=agasc_file, date='2000:001', fix_color1=False) stars1.sort('AGASC_ID') @@ -336,7 +348,7 @@ def mp_get_agascid(agasc_id): @pytest.mark.skipif('not TEST_ASCDS') @pytest.mark.parametrize("ra,dec", list(zip(RAS[:2], DECS[:2]))) def test_agasc_id(ra, dec, radius=0.2, nstar_limit=5): - agasc_file = AGASC_FILE[DS_AGASC_VERSION] + agasc_file = agasc.get_agasc_filename("miniagasc_*", version=DS_AGASC_VERSION) print('ra, dec =', ra, dec) stars = agasc.get_agasc_cone(ra, dec, radius=radius, agasc_file=agasc_file, @@ -355,15 +367,14 @@ def test_agasc_id(ra, dec, radius=0.2, nstar_limit=5): def test_proseco_agasc_1p7(): - proseco_file = DATA_DIR / 'proseco_agasc_1p7.h5' - if not proseco_file.exists(): - pytest.skip(f'No proseco agasc file {proseco_file} found') + proseco_file = agasc.get_agasc_filename("proseco_agasc_*", version="1p7") + mini_file = agasc.get_agasc_filename("miniagasc_*", version="1p7") # Stars looking toward galactic center (dense!) p_stars = agasc.get_agasc_cone(-266, -29, 3, agasc_file=proseco_file, date='2000:001') m_stars = agasc.get_agasc_cone(-266, -29, 3, - agasc_file=AGASC_FILE['1p7'], date='2000:001') + agasc_file=mini_file, date='2000:001') # Every miniagasc_1p7 star is in proseco_agasc_1p7 m_ids = m_stars['AGASC_ID'] @@ -382,8 +393,12 @@ def test_proseco_agasc_1p7(): @pytest.mark.skipif(NO_MAGS_IN_SUPPLEMENT, reason='no mags in supplement') def test_supplement_get_agasc_cone(): ra, dec = 282.53, -0.38 # Obsid 22429 with a couple of color1=1.5 stars - stars1 = agasc.get_agasc_cone(ra, dec, date='2021:001', use_supplement=False) - stars2 = agasc.get_agasc_cone(ra, dec, date='2021:001', use_supplement=True) + stars1 = agasc.get_agasc_cone( + ra, dec, date='2021:001', agasc_file=MINIAGASC, use_supplement=False + ) + stars2 = agasc.get_agasc_cone( + ra, dec, date='2021:001', agasc_file=MINIAGASC, use_supplement=True + ) ok = stars2['MAG_CATID'] == agasc.MAG_CATID_SUPPLEMENT change_names = ['MAG_CATID', 'COLOR1', 'MAG_ACA', 'MAG_ACA_ERR'] @@ -420,8 +435,8 @@ def test_supplement_get_star(): agasc_id = 58720672 # Also checks that the default is False given the os.environ override for # this test file. - star1 = agasc.get_star(agasc_id) - star2 = agasc.get_star(agasc_id, use_supplement=True) + star1 = agasc.get_star(agasc_id, agasc_file=MINIAGASC) + star2 = agasc.get_star(agasc_id, agasc_file=MINIAGASC, use_supplement=True) assert star1['MAG_CATID'] != agasc.MAG_CATID_SUPPLEMENT assert star2['MAG_CATID'] == agasc.MAG_CATID_SUPPLEMENT @@ -463,8 +478,8 @@ def test_supplement_get_star_disable_decorator(): @pytest.mark.skipif(NO_MAGS_IN_SUPPLEMENT, reason='no mags in supplement') def test_supplement_get_stars(): agasc_ids = [58720672, 670303120] - star1 = agasc.get_stars(agasc_ids) - star2 = agasc.get_stars(agasc_ids, use_supplement=True) + star1 = agasc.get_stars(agasc_ids, agasc_file=MINIAGASC) + star2 = agasc.get_stars(agasc_ids, agasc_file=MINIAGASC, use_supplement=True) assert np.all(star1['MAG_CATID'] != agasc.MAG_CATID_SUPPLEMENT) assert np.all(star2['MAG_CATID'] == agasc.MAG_CATID_SUPPLEMENT) diff --git a/agasc/tests/test_agasc_healpix.py b/agasc/tests/test_agasc_healpix.py index 5097e976..eaa3d1ba 100644 --- a/agasc/tests/test_agasc_healpix.py +++ b/agasc/tests/test_agasc_healpix.py @@ -5,43 +5,31 @@ """ import numpy as np import pytest -import tables -from astropy.table import Table -from ska_helpers.utils import temp_env_var import agasc from agasc.healpix import get_healpix_info -# Look for any AGASC 1.8 files in the default AGASC dir, potentially including release -# candidate files. This is to allow testing prior to the final release of AGASC 1.8. -def get_agasc_1p8(): - root = "proseco_agasc" - agasc_files = agasc.default_agasc_dir().glob(f"{root}_1p8*.h5") - paths = {path.name: path for path in agasc_files} - if paths: - if f"{root}_1p8.h5" in paths: - agasc_file = paths[f"{root}_1p8.h5"] - else: - # Take the last one alphabetically which works for rc < 10 - name = sorted(paths)[-1] - agasc_file = paths[name] +AGASC_FILES = {} +for root, version in [("proseco_agasc_*", "1p8"), ("agasc_healpix_*", "1p7")]: + try: + fn = agasc.get_agasc_filename(root, version=version, allow_rc=True) + except FileNotFoundError: + AGASC_FILES[root, version] = None else: - agasc_file = None - return agasc_file - - -AGASC_FILE_1P8 = get_agasc_1p8() - -if AGASC_FILE_1P8 is None: - pytest.skip("No proseco_agasc 1.8 file found", allow_module_level=True) + AGASC_FILES[root, version] = fn +@pytest.mark.skipif( + AGASC_FILES["proseco_agasc_*", "1p8"] is None, + reason="No AGASC 1.8 file found", +) def test_healpix_index(): """ Test that the healpix index table is present and has the right number of rows. """ - healpix_index_map, nside = get_healpix_info(AGASC_FILE_1P8) + agasc_file = AGASC_FILES["proseco_agasc_*", "1p8"] + healpix_index_map, nside = get_healpix_info(agasc_file) assert len(healpix_index_map) == 12 * nside**2 @@ -51,16 +39,25 @@ def test_healpix_index(): decs = [-40] +@pytest.mark.skipif( + AGASC_FILES["agasc_healpix_*", "1p7"] is None, + reason="No AGASC 1.7 HEALpix file found", +) @pytest.mark.parametrize("ra, dec", zip(ras, decs)) def test_get_agasc_cone(ra, dec): stars1p7 = agasc.get_agasc_cone( ra, dec, radius=0.2, - agasc_file=agasc.default_agasc_dir() / "proseco_agasc_1p7.h5", - date="1997:001", + agasc_file=agasc.get_agasc_filename("agasc*", version="1p7"), + date="2023:001", ) - stars1p8 = agasc.get_agasc_cone( - ra, dec, radius=0.2, agasc_file=AGASC_FILE_1P8, date="1997:001" + + stars1p7_healpix = agasc.get_agasc_cone( + ra, + dec, + radius=0.2, + agasc_file=AGASC_FILES["agasc_healpix_*", "1p7"], + date="2023:001", ) - assert np.all(stars1p7["AGASC_ID"] == stars1p8["AGASC_ID"]) + assert np.all(stars1p7["AGASC_ID"] == stars1p7_healpix["AGASC_ID"])