From 3bf4367adc07adc8dadf19eff2009968f946556b Mon Sep 17 00:00:00 2001 From: Jost Migenda Date: Tue, 30 Apr 2024 20:44:47 +0100 Subject: [PATCH 1/5] update get_models() to use modern downloader * will now download to `model_path` instead of current directory * deprecate `download_dir` argument, which was never officially documented --- python/snewpy/__init__.py | 43 ++++++++++----------------------------- 1 file changed, 11 insertions(+), 32 deletions(-) diff --git a/python/snewpy/__init__.py b/python/snewpy/__init__.py index 407c6f8f7..f1b04dd40 100644 --- a/python/snewpy/__init__.py +++ b/python/snewpy/__init__.py @@ -8,7 +8,6 @@ """ from ._version import __version__ -from pathlib import Path from sys import exit import os @@ -33,36 +32,29 @@ def get_models(models=None, download_dir='SNEWPY_models'): Local directory to download model files to. """ from concurrent.futures import ThreadPoolExecutor, as_completed - from ._model_urls import model_urls - from ._model_downloader import _download as download + from .models.registry_model import all_models - for model in list(model_urls): - if model_urls[model] == []: - del model_urls[model] - continue + all_models = {m.__name__: m for m in all_models} + all_model_names = sorted(all_models.keys()) if models == "all": - models = model_urls.keys() + models = all_model_names elif isinstance(models, str): models = [models] elif models == None: # Select model(s) to download - print(f"Available models in this version of SNEWPY: {list(model_urls.keys())}") - if not model_urls: - print("Error: `get_models()` only works after installing SNEWPY via `pip install snewpy`. " - "If you have cloned the git repo, model files are available in the `models/` folder.") - return False + print(f"Available models in SNEWPY v{__version__}: {all_model_names}") selected = input("\nType a model name, 'all' to download all models or to cancel: ").strip() if selected == "all": - models = model_urls.keys() + models = all_model_names elif selected == "": exit() - elif selected in model_urls.keys(): + elif selected in all_model_names: models = [selected] while True: selected = input("\nType another model name or if you have selected all models you want to download: ").strip() - if selected in model_urls.keys(): + if selected in all_model_names: models.append(selected) elif selected == "": break @@ -74,25 +66,12 @@ def get_models(models=None, download_dir='SNEWPY_models'): print(f"\nYou have selected the models: {models}\n") - # Download model files - if not os.path.isdir(download_dir): - print(f"Creating directory '{download_dir}' ...") - os.makedirs(download_dir) - pool = ThreadPoolExecutor(max_workers=8) results = [] for model in models: - model_dir = download_dir + '/' + model - print(f"Downloading files for '{model}' into '{model_dir}' ...") - - for url in model_urls[model]: - local_file = model_dir + url.split(model, maxsplit=1)[1] - if os.path.exists(local_file) and local_file.find('README') == -1 and local_file.find('.ipynb') == -1: - print(f"File '{local_file}' already exists. Skipping download.") - else: - if not os.path.isdir(os.path.dirname(local_file)): - os.makedirs(os.path.dirname(local_file)) - results.append(pool.submit(download, src=url, dest=Path(local_file))) + print(f"Downloading files for '{model}' into '{model_path}' ...") + for progenitor in all_models[model].get_param_combinations(): + results.append(pool.submit(all_models[model], **progenitor)) exceptions = [] for result in as_completed(results): From 7f505d6bacda931c626f5ca1b272e8e43f1c6bfe Mon Sep 17 00:00:00 2001 From: Jost Migenda Date: Tue, 30 Apr 2024 20:46:37 +0100 Subject: [PATCH 2/5] Deprecate `download_dir` and minor get_models improvements --- python/snewpy/__init__.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/python/snewpy/__init__.py b/python/snewpy/__init__.py index f1b04dd40..a18117832 100644 --- a/python/snewpy/__init__.py +++ b/python/snewpy/__init__.py @@ -21,7 +21,7 @@ base_path = os.sep.join(src_path.split(os.sep)[:-2]) model_path = os.path.join(get_cache_dir(), 'snewpy/models') -def get_models(models=None, download_dir='SNEWPY_models'): +def get_models(models=None, download_dir=None): """Download model files from the snewpy repository. Parameters @@ -29,11 +29,15 @@ def get_models(models=None, download_dir='SNEWPY_models'): models : list or str Models to download. Can be 'all', name of a single model or list of model names. download_dir : str - Local directory to download model files to. + [Deprecated, do not use.] """ from concurrent.futures import ThreadPoolExecutor, as_completed + from warnings import warn from .models.registry_model import all_models + if download_dir is not None: + warn("The `download_dir` argument to `get_models` is deprecated and will be removed soon.", FutureWarning, stacklevel=2) + all_models = {m.__name__: m for m in all_models} all_model_names = sorted(all_models.keys()) @@ -41,7 +45,7 @@ def get_models(models=None, download_dir='SNEWPY_models'): models = all_model_names elif isinstance(models, str): models = [models] - elif models == None: + elif models is None: # Select model(s) to download print(f"Available models in SNEWPY v{__version__}: {all_model_names}") @@ -51,11 +55,11 @@ def get_models(models=None, download_dir='SNEWPY_models'): elif selected == "": exit() elif selected in all_model_names: - models = [selected] + models = {selected} while True: selected = input("\nType another model name or if you have selected all models you want to download: ").strip() if selected in all_model_names: - models.append(selected) + models.add(selected) elif selected == "": break else: @@ -68,8 +72,8 @@ def get_models(models=None, download_dir='SNEWPY_models'): pool = ThreadPoolExecutor(max_workers=8) results = [] + print(f"Downloading files for {models} to '{model_path}' ...") for model in models: - print(f"Downloading files for '{model}' into '{model_path}' ...") for progenitor in all_models[model].get_param_combinations(): results.append(pool.submit(all_models[model], **progenitor)) From d339cb0a423851fad285461c7664d59c9ff6e51e Mon Sep 17 00:00:00 2001 From: Jost Migenda Date: Tue, 30 Apr 2024 20:50:59 +0100 Subject: [PATCH 3/5] remove _get_model_urls helper function --- .github/workflows/publish.yml | 5 +---- python/snewpy/__init__.py | 26 -------------------------- python/snewpy/_model_urls.py | 6 ------ 3 files changed, 1 insertion(+), 36 deletions(-) delete mode 100644 python/snewpy/_model_urls.py diff --git a/.github/workflows/publish.yml b/.github/workflows/publish.yml index 807c297e8..fac2c05cb 100644 --- a/.github/workflows/publish.yml +++ b/.github/workflows/publish.yml @@ -27,11 +27,8 @@ jobs: TWINE_USERNAME: __token__ TWINE_PASSWORD: ${{ secrets.PYPI_JM }} run: | - python -c 'import python.snewpy; python.snewpy._get_model_urls()' - cat python/snewpy/_model_urls.py pip install . - python -c 'import snewpy; snewpy.get_models(models="Bollig_2016")' - rm -r SNEWPY_models/ + python -c 'import snewpy; print(snewpy.__version__)' python setup.py sdist bdist_wheel twine upload dist/* diff --git a/python/snewpy/__init__.py b/python/snewpy/__init__.py index a18117832..367a9522b 100644 --- a/python/snewpy/__init__.py +++ b/python/snewpy/__init__.py @@ -86,29 +86,3 @@ def get_models(models=None, download_dir=None): print("Please check your internet connection and try again later. If this persists, please report it at https://github.com/SNEWS2/snewpy/issues") exit(1) pool.shutdown(wait=False) - - -def _get_model_urls(): - """List URLs of model files for the current release. - - When building a snewpy release, generate a dictionary of available models - and the URLs at which the respective files are located. Users can then use - get_models() to interactively select which model(s) to download. - """ - - repo_dir = os.path.normpath(os.path.dirname(os.path.abspath(__file__)) + '/../../') - url_base = 'https://github.com/SNEWS2/snewpy/raw/v' + __version__ - - with open(os.path.dirname(os.path.abspath(__file__)) + '/_model_urls.py', 'w') as f: - f.write('model_urls = {\n') - for model in sorted(os.listdir(repo_dir + '/models')): - urls = [] - for root, dirs, files in os.walk(repo_dir + '/models/' + model): - for file in files: - urls.append(f'{url_base}{root[len(repo_dir):]}/{file}') - - f.write(f' "{model}": [\n') - for url in sorted(urls): - f.write(f' "{url}",\n') - f.write(' ],\n') - f.write('}\n') diff --git a/python/snewpy/_model_urls.py b/python/snewpy/_model_urls.py deleted file mode 100644 index a80525a50..000000000 --- a/python/snewpy/_model_urls.py +++ /dev/null @@ -1,6 +0,0 @@ -"""Dictionary of models and URLs where model files are available to download. -When preparing a snewpy release, this dictionary should be automatically -generated using _get_model_urls() in __init__.py. Do NOT edit this manually. -""" - -model_urls = {} \ No newline at end of file From bcd930607642dea83295816c75fc6e175988cd02 Mon Sep 17 00:00:00 2001 From: Jost Migenda Date: Tue, 30 Apr 2024 20:59:39 +0100 Subject: [PATCH 4/5] remove unnecessary use of get_models in undocumented internal function See #225 --- python/snewpy/models/__init__.py | 20 ++------------------ 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/python/snewpy/models/__init__.py b/python/snewpy/models/__init__.py index 57f1c8771..b8569918a 100644 --- a/python/snewpy/models/__init__.py +++ b/python/snewpy/models/__init__.py @@ -1,7 +1,5 @@ -import logging from warnings import warn -from snewpy import get_models, model_path from . import ccsn, presn @@ -12,18 +10,13 @@ def __getattr__(name): raise AttributeError(f"module {__name__} has no attribute {name}") -def _init_model(model_name, download=True, download_dir=model_path, **user_param): +def _init_model(model_name, **user_param): """Attempts to retrieve instantiated SNEWPY model using model class name and model parameters. - If a model name is valid, but is not found and `download`=True, this function will attempt to download the model Parameters ---------- model_name : str Name of SNEWPY model to import, must exactly match the name of the corresponding model class - download : bool - Switch for attempting to download model data if the first load attempt failed due to a missing file. - download_dir : str - Local directory to download model files to. user_param : varies User-requested model parameters used to initialize the model, if one is found. Error checking is performed during model initialization @@ -57,13 +50,4 @@ def _init_model(model_name, download=True, download_dir=model_path, **user_param else: raise ValueError(f"Unable to find model with name '{model_name}' in snewpy.models.ccsn or snewpy.models.presn") - try: - return getattr(module, model_name)(**user_param) - except FileNotFoundError as e: - logger = logging.getLogger() - logger.warning(f"Unable to find model {model_name} in {download_dir}") - if not download: - raise e - logger.warning(f"Attempting to download model...") - get_models(model_name, download_dir) - return getattr(module, model_name)(**user_param) + return getattr(module, model_name)(**user_param) From 59fb68bd1c9f0ec75cdb80dda507411a5ebfdbc0 Mon Sep 17 00:00:00 2001 From: Jost Migenda Date: Tue, 30 Apr 2024 21:36:29 +0100 Subject: [PATCH 5/5] Delete test for _model_urls --- python/snewpy/test/test_02_models.py | 7 ------- 1 file changed, 7 deletions(-) diff --git a/python/snewpy/test/test_02_models.py b/python/snewpy/test/test_02_models.py index 304fdcd2c..c66387c26 100644 --- a/python/snewpy/test/test_02_models.py +++ b/python/snewpy/test/test_02_models.py @@ -8,7 +8,6 @@ Sukhbold_2015, Bollig_2016, Walk_2018, \ Walk_2019, Fornax_2019, Warren_2020, \ Kuroda_2020, Fornax_2021, Zha_2021 -from snewpy._model_urls import model_urls from astropy import units as u from snewpy import model_path import os @@ -16,12 +15,6 @@ class TestModels(unittest.TestCase): - def test_model_urls(self): - """Test that snewpy._model_urls.model_urls is empty. This should be populated if snewpy is downloaded from PyPI. - This serves as a guard against accidentally committing/merging a populated model_urls to main. - """ - self.assertFalse(model_urls) - def test_Nakazato_2013(self): """ Instantiate a set of 'Nakazato 2013' models