From 8bb4f3138d19228e41b60397a6c31bf8588eefd4 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Mon, 19 Feb 2024 11:25:32 -0500 Subject: [PATCH 01/14] ci(release): set version to 1.5.0.dev0, touch up HISTORY.md --- HISTORY.md | 11 +++-------- docs/conf.py | 2 +- modflow_devtools/__init__.py | 2 +- version.txt | 2 +- 4 files changed, 6 insertions(+), 11 deletions(-) diff --git a/HISTORY.md b/HISTORY.md index f927df2..5ee3c8e 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -2,18 +2,13 @@ #### New features -* [feat(Executables)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/3129417dae2de3aece80c8056a2ac50eede56b91): Support collection-style membership test (#131). Committed by wpbonelli on 2023-12-18. -* [feat](https://github.com/MODFLOW-USGS/modflow-devtools/commit/6728859a984a3080f8fd4f1135de36bc17454098): Add latex and plot style utilities (#132). Committed by wpbonelli on 2024-01-09. +* [feat(latex)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/6728859a984a3080f8fd4f1135de36bc17454098): Add latex utilities (#132). Committed by wpbonelli on 2024-01-09. * [feat(misc)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/a9b801932866a26a996ed3a45f16048b15246472): Parse literals from environment variables (#135). Committed by wpbonelli on 2024-01-21. -* [feat(ostags)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/0ad10751ea6ce752e59d83e8cd6275906d73fa70): Apple silicon (#139). Committed by wpbonelli on 2024-02-18. - -#### Bug fixes - -* [fix](https://github.com/MODFLOW-USGS/modflow-devtools/commit/fd215000c6215b0891e78ee621e40abb2a20b28a): Drop plot styles (already in flopy) (#133). Committed by wpbonelli on 2024-01-09. +* [feat(ostags)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/0ad10751ea6ce752e59d83e8cd6275906d73fa70): add OS tags for Apple silicon (#139). Committed by wpbonelli on 2024-02-18. #### Refactoring -* [refactor](https://github.com/MODFLOW-USGS/modflow-devtools/commit/9356e067ea813aeeeda2582cf7ec174c11d80159): Remove executables module/class (#136). Committed by wpbonelli on 2024-01-25. +* [refactor](https://github.com/MODFLOW-USGS/modflow-devtools/commit/9356e067ea813aeeeda2582cf7ec174c11d80159): Remove executables module/class (#136). Committed by wpbonelli on 2024-01-25. Should be in a major release per semver, but nothing is using it, so this should be safe. * [refactor(fixtures)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/613ad010ff6fc782f231b7fa21d1cc660732e7be): Support pytest>=8, drop pytest-cases dependency (#137). Committed by wpbonelli on 2024-01-31. ### Version 1.3.1 diff --git a/docs/conf.py b/docs/conf.py index 127aeb0..1d0f770 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -8,7 +8,7 @@ project = "modflow-devtools" author = "MODFLOW Team" -release = "1.4.0" +release = '1.5.0.dev0' # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration diff --git a/modflow_devtools/__init__.py b/modflow_devtools/__init__.py index 80b4f29..f8f2600 100644 --- a/modflow_devtools/__init__.py +++ b/modflow_devtools/__init__.py @@ -1,6 +1,6 @@ __author__ = "Joseph D. Hughes" __date__ = "Feb 19, 2024" -__version__ = "1.4.0" +__version__ = "1.5.0.dev0" __maintainer__ = "Joseph D. Hughes" __email__ = "jdhughes@usgs.gov" __status__ = "Production" diff --git a/version.txt b/version.txt index e21e727..47ed95f 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -1.4.0 \ No newline at end of file +1.5.0.dev0 \ No newline at end of file From 827b5ec63ebe0b9ea833957637d6b60fdc2f3198 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Sun, 25 Feb 2024 12:38:57 -0500 Subject: [PATCH 02/14] refactor(latex): support path-like, add docstrings (#142) * add docs and support str or path-like for latex utils * also install nightly build and update flopy pkgs in ci --- .github/workflows/ci.yml | 10 ++++++- docs/conf.py | 2 +- modflow_devtools/latex.py | 62 ++++++++++++++++++++++++++++++++++----- 3 files changed, 65 insertions(+), 9 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 549431c..26cbc9e 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -120,6 +120,11 @@ jobs: - name: Install executables uses: modflowpy/install-modflow-action@v1 + + - name: Install nightly build + uses: modflowpy/install-modflow-action@v1 + with: + repo: modflow6-nightly-build - name: Setup GNU Fortran ${{ env.GCC_V }} uses: awvwgk/setup-fortran@main @@ -137,7 +142,7 @@ jobs: run: | pip install . pip install ".[test]" - + - name: Cache modflow6 examples id: cache-examples uses: actions/cache@v3 @@ -152,6 +157,9 @@ jobs: pip install -r requirements.pip.txt pip install -r requirements.usgs.txt + - name: Update FloPy packages + run: python -m flopy.mf6.utils.generate_classes --ref develop --no-backup + - name: Build modflow6 example models if: steps.cache-examples.outputs.cache-hit != 'true' working-directory: modflow6-examples/autotest diff --git a/docs/conf.py b/docs/conf.py index 1d0f770..9fd38eb 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -8,7 +8,7 @@ project = "modflow-devtools" author = "MODFLOW Team" -release = '1.5.0.dev0' +release = "1.5.0.dev0" # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration diff --git a/modflow_devtools/latex.py b/modflow_devtools/latex.py index 5d6f5eb..bbe766c 100644 --- a/modflow_devtools/latex.py +++ b/modflow_devtools/latex.py @@ -1,13 +1,38 @@ -import os +from os import PathLike +from pathlib import Path +from typing import Iterable, Union -def build_table(caption, fpth, arr, headings=None, col_widths=None): +def build_table( + caption: str, + fpth: Union[str, PathLike], + arr, + headings: Iterable[str] = None, + col_widths: Iterable[float] = None, +): + """ + Build a LaTeX table from the given NumPy array. + + Parameters + ---------- + caption : str + The table's caption + fpth : str or path-like + The LaTeX file to create + arr : numpy recarray + The array + headings : iterable of str + The table headings + col_widths : iterable of float + The table's column widths + """ + + fpth = Path(fpth).expanduser().absolute().with_suffix(".tex") + if headings is None: headings = arr.dtype.names ncols = len(arr.dtype.names) - if not fpth.endswith(".tex"): - fpth += ".tex" - label = "tab:{}".format(os.path.basename(fpth).replace(".tex", "")) + label = "tab:{}".format(fpth.stem) line = get_header(caption, label, headings, col_widths=col_widths) @@ -29,8 +54,32 @@ def build_table(caption, fpth, arr, headings=None, col_widths=None): def get_header( - caption, label, headings, col_widths=None, center=True, firsthead=False + caption: str, + label: str, + headings: Iterable[str], + col_widths: Iterable[float] = None, + center: bool = True, + firsthead: bool = False, ): + """ + Build a LaTeX table header. + + Parameters + ---------- + caption : str + The table's caption + label : str + The table's label + headings : iterable of str + The table's heading + col_widths : iterable of float + The table's column widths + center : bool + Whether to center-align the table text + firsthead : bool + Whether to add first header + """ + ncol = len(headings) if col_widths is None: dx = 0.8 / float(ncol) @@ -85,7 +134,6 @@ def exp_format(v): s = f"{v:.2e}" s = s.replace("e-0", "e-") s = s.replace("e+0", "e+") - # s = s.replace("e", " \\times 10^{") + "}$" return s From 9403562c72f0a4ca3a09889b0dfcc8f4640be19e Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Tue, 5 Mar 2024 11:11:46 -0500 Subject: [PATCH 03/14] chore(dependencies): restrict yanked pytest version 8.1.0 (#143) * pytest 8.1.0 broke some plugins and was yanked * https://github.com/pytest-dev/pytest/releases/tag/8.1.0 * always install examples dependencies in ci (need flopy) --- .github/workflows/ci.yml | 1 - pyproject.toml | 2 +- 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 26cbc9e..1aa17be 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -151,7 +151,6 @@ jobs: key: modflow6-examples-${{ hashFiles('modflow6-examples/data/**') }} - name: Install extra Python packages - if: steps.cache-examples.outputs.cache-hit != 'true' working-directory: modflow6-examples/etc run: | pip install -r requirements.pip.txt diff --git a/pyproject.toml b/pyproject.toml index 3ed78e8..16c2018 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -58,7 +58,7 @@ test = [ "meson!=0.63.0", "ninja", "numpy", - "pytest", + "pytest!=8.1.0", "pytest-cov", "pytest-dotenv", "pytest-xdist", From 5c0ee3710c750d0013dd643a7cf418971d1182ad Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Mon, 11 Mar 2024 14:41:09 -0400 Subject: [PATCH 04/14] docs(fixtures): updated outdated mf6 test script reference (#144) --- docs/md/fixtures.md | 31 ++++++------------------------- 1 file changed, 6 insertions(+), 25 deletions(-) diff --git a/docs/md/fixtures.md b/docs/md/fixtures.md index 750e7fd..b847f10 100644 --- a/docs/md/fixtures.md +++ b/docs/md/fixtures.md @@ -133,34 +133,22 @@ External model test cases can be filtered by model name or by the packages the m Filtering models by name is functionally equivalent to filtering `pytest` cases with `-k`. (In the former case the filter is applied before test collection, while the latter collects tests as usual and then applies the filter.) -With no filtering, collecting models from the `modflow6-largetestmodels` repo: +For instance, running the `test_largetestmodels.py` script in the `MODFLOW-USGS/modflow6` repository's `autotest/` folder, and selecting a particular model from the `MODFLOW-USGS/largetestmodels` repository by name: ```shell -autotest % pytest -v test_z03_largetestmodels.py --collect-only -... -collected 18 items -``` - -Selecting a particular model by name: - -```shell -autotest % pytest -v test_z03_largetestmodels.py --collect-only --model test1002_biscqtg_disv_gnc_nr_dev +autotest % pytest -v test_largetestmodels.py --collect-only --model test1002_biscqtg_disv_gnc_nr_dev ... collected 1 item - - - +... ``` Equivalently: ```shell -autotest % pytest -v test_z03_largetestmodels.py --collect-only -k test1002_biscqtg_disv_gnc_nr_dev +autotest % pytest -v test_largetestmodels.py --collect-only -k test1002_biscqtg_disv_gnc_nr_dev ... collected 18 items / 17 deselected / 1 selected - - - +... ``` The `--model` option can be used multiple times, e.g. `--model --model `. @@ -170,14 +158,7 @@ The `--model` option can be used multiple times, e.g. `--model --model MODFLOW 6 models from external repos can also be filtered by packages used. For instance, to select only large GWT models: ```shell -autotest % pytest -v test_z03_largetestmodels.py --collect-only --package gwt -... -collected 3 items - - - - - +autotest % pytest -v --package gwt ``` ### Utility functions From 70cce0e25be7dc306a14511ff401fd91977118f9 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Mon, 11 Mar 2024 16:38:54 -0400 Subject: [PATCH 05/14] chore: switch to ruff, misc linting fixes (#145) * switch to Ruff from black, isort, pylint, flake8 * apply some fixes recommended by ruff's linter * reformat python files with ruff's default ruleset * add ruff lint rule npy201 --- .github/workflows/ci.yml | 26 +++++----------- autotest/test_build.py | 4 +-- autotest/test_download.py | 8 ++--- autotest/test_fixtures.py | 15 +++------- autotest/test_markers.py | 26 +++++++++++++--- autotest/test_misc.py | 24 +++++---------- modflow_devtools/download.py | 58 ++++++++++++++---------------------- modflow_devtools/fixtures.py | 22 ++++---------- modflow_devtools/imports.py | 4 +-- modflow_devtools/latex.py | 6 +--- modflow_devtools/markers.py | 12 +++----- modflow_devtools/misc.py | 56 +++++++++++++--------------------- modflow_devtools/ostags.py | 4 +-- pyproject.toml | 30 ++++++++----------- scripts/update_version.py | 27 +++++------------ 15 files changed, 120 insertions(+), 202 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 1aa17be..e9fea5f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -30,21 +30,10 @@ jobs: cache-dependency-path: pyproject.toml - name: Install Python packages - run: | - pip install . - pip install ".[lint]" - - - name: Run isort - run: isort --verbose --check --diff modflow_devtools - - - name: Run black - run: black --check --diff modflow_devtools + run: pip install ".[lint]" - - name: Run flake8 - run: flake8 --count --show-source --exit-zero modflow_devtools - - - name: Run pylint - run: pylint --jobs=0 --errors-only --exit-zero modflow_devtools + - name: Run ruff + run: ruff check . build: name: Build @@ -139,11 +128,10 @@ jobs: - name: Install Python packages working-directory: modflow-devtools - run: | - pip install . - pip install ".[test]" + run: pip install ".[test]" - name: Cache modflow6 examples + if: matrix.python != 3.8 id: cache-examples uses: actions/cache@v3 with: @@ -151,16 +139,18 @@ jobs: key: modflow6-examples-${{ hashFiles('modflow6-examples/data/**') }} - name: Install extra Python packages + if: matrix.python != 3.8 working-directory: modflow6-examples/etc run: | pip install -r requirements.pip.txt pip install -r requirements.usgs.txt - name: Update FloPy packages + if: matrix.python != 3.8 run: python -m flopy.mf6.utils.generate_classes --ref develop --no-backup - name: Build modflow6 example models - if: steps.cache-examples.outputs.cache-hit != 'true' + if: matrix.python != 3.8 && steps.cache-examples.outputs.cache-hit != 'true' working-directory: modflow6-examples/autotest run: pytest -v -n auto test_scripts.py --init diff --git a/autotest/test_build.py b/autotest/test_build.py index 36697f7..4effb9c 100644 --- a/autotest/test_build.py +++ b/autotest/test_build.py @@ -16,9 +16,7 @@ _system = platform.system() _exe_ext = ".exe" if _system == "Windows" else "" _lib_ext = ( - ".so" - if _system == "Linux" - else (".dylib" if _system == "Darwin" else ".dll") + ".so" if _system == "Linux" else (".dylib" if _system == "Darwin" else ".dll") ) diff --git a/autotest/test_download.py b/autotest/test_download.py index 9d5173d..dc3b43b 100644 --- a/autotest/test_download.py +++ b/autotest/test_download.py @@ -21,9 +21,7 @@ @pytest.mark.parametrize("retries", [-1, 0, 1.5]) def test_get_releases_bad_params(per_page, retries): with pytest.raises(ValueError): - get_releases( - "executables", per_page=per_page, retries=retries, verbose=True - ) + get_releases("executables", per_page=per_page, retries=retries, verbose=True) @flaky @@ -109,9 +107,7 @@ def test_download_and_unzip(function_tmpdir, delete_zip): zip_name = "mf6.3.0_linux.zip" dir_name = zip_name.replace(".zip", "") url = f"https://github.com/MODFLOW-USGS/modflow6/releases/download/6.3.0/{zip_name}" - download_and_unzip( - url, function_tmpdir, delete_zip=delete_zip, verbose=True - ) + download_and_unzip(url, function_tmpdir, delete_zip=delete_zip, verbose=True) assert (function_tmpdir / zip_name).is_file() != delete_zip diff --git a/autotest/test_fixtures.py b/autotest/test_fixtures.py index dcd7122..efddf54 100644 --- a/autotest/test_fixtures.py +++ b/autotest/test_fixtures.py @@ -39,8 +39,7 @@ def test_function_scoped_tmpdir_slash_in_name(function_tmpdir, name): replaced1 = name.replace("/", "_").replace("\\", "_").replace(":", "_") replaced2 = name.replace("/", "_").replace("\\", "__").replace(":", "_") assert ( - f"{inspect.currentframe().f_code.co_name}[{replaced1}]" - in function_tmpdir.stem + f"{inspect.currentframe().f_code.co_name}[{replaced1}]" in function_tmpdir.stem or f"{inspect.currentframe().f_code.co_name}[{replaced2}]" in function_tmpdir.stem ) @@ -144,9 +143,7 @@ def test_keep_class_scoped_tmpdir(tmp_path, arg): ] assert pytest.main(args) == ExitCode.OK assert Path( - tmp_path - / f"{TestKeepClassScopedTmpdirInner.__name__}0" - / test_keep_fname + tmp_path / f"{TestKeepClassScopedTmpdirInner.__name__}0" / test_keep_fname ).is_file() @@ -165,9 +162,7 @@ def test_keep_module_scoped_tmpdir(tmp_path, arg): ] assert pytest.main(args) == ExitCode.OK this_path = Path(__file__) - keep_path = ( - tmp_path / f"{str(this_path.parent.name)}.{str(this_path.stem)}0" - ) + keep_path = tmp_path / f"{str(this_path.parent.name)}.{str(this_path.stem)}0" assert test_keep_fname in [f.name for f in keep_path.glob("*")] @@ -206,9 +201,7 @@ def test_keep_failed_function_scoped_tmpdir(function_tmpdir, keep): args += ["--keep-failed", function_tmpdir] assert pytest.main(args) == ExitCode.TESTS_FAILED - kept_file = Path( - function_tmpdir / f"{inner_fn}0" / test_keep_fname - ).is_file() + kept_file = Path(function_tmpdir / f"{inner_fn}0" / test_keep_fname).is_file() assert kept_file if keep else not kept_file diff --git a/autotest/test_markers.py b/autotest/test_markers.py index cd105c4..e93020b 100644 --- a/autotest/test_markers.py +++ b/autotest/test_markers.py @@ -3,10 +3,24 @@ from shutil import which from packaging.version import Version - -from modflow_devtools.markers import * +import pytest + +from modflow_devtools.markers import ( + requires_exe, + require_exe, + requires_program, + require_program, + requires_pkg, + require_package, + requires_platform, + require_platform, + excludes_platform, + requires_python, + require_python, +) exe = "pytest" +pkg = exe @requires_exe(exe) @@ -14,6 +28,7 @@ def test_require_exe(): assert which(exe) require_exe(exe) require_program(exe) + requires_program(exe) exes = [exe, "python"] @@ -24,14 +39,15 @@ def test_require_exe_multiple(): assert all(which(exe) for exe in exes) -@requires_pkg("pytest") +@requires_pkg(pkg) def test_requires_pkg(): import numpy assert numpy is not None + require_package(pkg) -@requires_pkg("pytest", "pluggy") +@requires_pkg(pkg, "pluggy") def test_requires_pkg_multiple(): import pluggy import pytest @@ -42,6 +58,7 @@ def test_requires_pkg_multiple(): @requires_platform("Windows") def test_requires_platform(): assert system() == "Windows" + require_platform("Windows") @excludes_platform("Darwin", ci_only=True) @@ -57,3 +74,4 @@ def test_requires_platform_ci_only(): def test_requires_python(version): if Version(py_ver) >= Version(version): assert requires_python(version) + assert require_python(version) diff --git a/autotest/test_misc.py b/autotest/test_misc.py index 283e5ff..e6c5b00 100644 --- a/autotest/test_misc.py +++ b/autotest/test_misc.py @@ -49,9 +49,7 @@ def test_set_env(): _repos_path = Path(__file__).parent.parent.parent.parent _repos_path = Path(_repos_path).expanduser().absolute() _testmodels_repo_path = _repos_path / "modflow6-testmodels" -_testmodels_repo_paths_mf6 = sorted( - list((_testmodels_repo_path / "mf6").glob("test*")) -) +_testmodels_repo_paths_mf6 = sorted(list((_testmodels_repo_path / "mf6").glob("test*"))) _testmodels_repo_paths_mf5to6 = sorted( list((_testmodels_repo_path / "mf5to6").glob("test*")) ) @@ -60,9 +58,7 @@ def test_set_env(): _examples_repo_path = _repos_path / "modflow6-examples" _examples_path = _examples_repo_path / "examples" _example_paths = ( - sorted(list(_examples_path.glob("ex-*"))) - if _examples_path.is_dir() - else [] + sorted(list(_examples_path.glob("ex-*"))) if _examples_path.is_dir() else [] ) @@ -150,9 +146,7 @@ def get_expected_namefiles(path, pattern="mfsim.nam") -> List[Path]: return sorted(list(set(folders))) -@pytest.mark.skipif( - not any(_example_paths), reason="modflow6-examples repo not found" -) +@pytest.mark.skipif(not any(_example_paths), reason="modflow6-examples repo not found") def test_get_model_paths_examples(): expected_paths = get_expected_model_dirs(_examples_path) paths = get_model_paths(_examples_path) @@ -193,9 +187,7 @@ def test_get_model_paths_exclude_patterns(models): assert len(paths) >= expected_count -@pytest.mark.skipif( - not any(_example_paths), reason="modflow6-examples repo not found" -) +@pytest.mark.skipif(not any(_example_paths), reason="modflow6-examples repo not found") def test_get_namefile_paths_examples(): expected_paths = get_expected_namefiles(_examples_path) paths = get_namefile_paths(_examples_path) @@ -287,9 +279,9 @@ def test_get_env(): assert get_env("NO_VALUE") is None with set_env(TEST_VALUE=str(True)): - assert get_env("NO_VALUE", True) == True - assert get_env("TEST_VALUE") == True - assert get_env("TEST_VALUE", default=False) == True + assert get_env("NO_VALUE", True) + assert get_env("TEST_VALUE") + assert get_env("TEST_VALUE", default=False) assert get_env("TEST_VALUE", default=1) == 1 with set_env(TEST_VALUE=str(1)): @@ -302,4 +294,4 @@ def test_get_env(): assert get_env("NO_VALUE", 1.1) == 1.1 assert get_env("TEST_VALUE") == 1.1 assert get_env("TEST_VALUE", default=2.1) == 1.1 - assert get_env("TEST_VALUE", default=False) == False + assert not get_env("TEST_VALUE", default=False) diff --git a/modflow_devtools/download.py b/modflow_devtools/download.py index fcf045b..719220d 100644 --- a/modflow_devtools/download.py +++ b/modflow_devtools/download.py @@ -58,10 +58,10 @@ def get_releases( """ if "/" not in repo: - raise ValueError(f"repo format must be owner/name") + raise ValueError("repo format must be owner/name") if not isinstance(retries, int) or retries < 1: - raise ValueError(f"retries must be a positive int") + raise ValueError("retries must be a positive int") params = {} if per_page is not None: @@ -96,9 +96,7 @@ def get_response_json(): # GitHub sometimes returns this error for valid URLs, so retry warn(f"URL request try {tries} failed ({err})") continue - raise RuntimeError( - f"cannot retrieve data from {req_url}" - ) from err + raise RuntimeError(f"cannot retrieve data from {req_url}") from err releases = [] max_pages = max_pages if max_pages else sys.maxsize @@ -132,13 +130,13 @@ def get_release(repo, tag="latest", retries=3, verbose=False) -> dict: """ if "/" not in repo: - raise ValueError(f"repo format must be owner/name") + raise ValueError("repo format must be owner/name") if not isinstance(tag, str) or not any(tag): - raise ValueError(f"tag must be a non-empty string") + raise ValueError("tag must be a non-empty string") if not isinstance(retries, int) or retries < 1: - raise ValueError(f"retries must be a positive int") + raise ValueError("retries must be a positive int") req_url = f"https://api.github.com/repos/{repo}" req_url = ( @@ -209,10 +207,10 @@ def get_latest_version(repo, retries=3, verbose=False) -> str: """ if "/" not in repo: - raise ValueError(f"repo format must be owner/name") + raise ValueError("repo format must be owner/name") if not isinstance(retries, int) or retries < 1: - raise ValueError(f"retries must be a positive int") + raise ValueError("retries must be a positive int") release = get_release(repo, retries=retries, verbose=verbose) return release["tag_name"] @@ -245,13 +243,13 @@ def get_release_assets( """ if "/" not in repo: - raise ValueError(f"repo format must be owner/name") + raise ValueError("repo format must be owner/name") if not isinstance(tag, str) or not any(tag): - raise ValueError(f"tag must be a non-empty string") + raise ValueError("tag must be a non-empty string") if not isinstance(retries, int) or retries < 1: - raise ValueError(f"retries must be a positive int") + raise ValueError("retries must be a positive int") release = get_release(repo, tag=tag, retries=retries, verbose=verbose) return ( @@ -292,21 +290,19 @@ def list_artifacts( """ if "/" not in repo: - raise ValueError(f"repo format must be owner/name") + raise ValueError("repo format must be owner/name") if not isinstance(retries, int) or retries < 1: - raise ValueError(f"retries must be a positive int") + raise ValueError("retries must be a positive int") - msg = f"artifact(s) for {repo}" + ( - f" matching name {name}" if name else "" - ) + msg = f"artifact(s) for {repo}" + (f" matching name {name}" if name else "") req_url = f"https://api.github.com/repos/{repo}/actions/artifacts" page = 1 params = {} if name is not None: if not isinstance(name, str) or len(name) == 0: - raise ValueError(f"name must be a non-empty string") + raise ValueError("name must be a non-empty string") params["name"] = name if per_page is not None: @@ -336,9 +332,7 @@ def get_response_json(): # GitHub sometimes returns this error for valid URLs, so retry warn(f"URL request try {tries} failed ({err})") continue - raise RuntimeError( - f"cannot retrieve data from {req_url}" - ) from err + raise RuntimeError(f"cannot retrieve data from {req_url}") from err artifacts = [] diff = 1 @@ -387,10 +381,10 @@ def download_artifact( """ if "/" not in repo: - raise ValueError(f"repo format must be owner/name") + raise ValueError("repo format must be owner/name") if not isinstance(retries, int) or retries < 1: - raise ValueError(f"retries must be a positive int") + raise ValueError("retries must be a positive int") req_url = f"https://api.github.com/repos/{repo}/actions/artifacts/{id}/zip" request = urllib.request.Request(req_url) @@ -415,9 +409,7 @@ def download_artifact( warn(f"URL request try {tries} failed ({err})") continue else: - raise RuntimeError( - f"cannot retrieve data from {req_url}" - ) from err + raise RuntimeError(f"cannot retrieve data from {req_url}") from err if verbose: print(f"Uncompressing: {zip_path}") @@ -496,14 +488,8 @@ def download_and_unzip( file_size = int(file_size) bfmt = "{:" + f"{len_file_size}" + ",d}" - sbfmt = ( - "{:>" - + f"{len(bfmt.format(int(file_size)))}" - + "s} bytes" - ) - print( - f" file size: {sbfmt.format(bfmt.format(int(file_size)))}" - ) + sbfmt = "{:>" + f"{len(bfmt.format(int(file_size)))}" + "s} bytes" + print(f" file size: {sbfmt.format(bfmt.format(int(file_size)))}") break except urllib.error.HTTPError as err: @@ -526,7 +512,7 @@ def download_and_unzip( # extract the files z.extractall(str(path)) - except: + except: # noqa: E722 p = "Could not unzip the file. Stopping." raise Exception(p) z.close() diff --git a/modflow_devtools/fixtures.py b/modflow_devtools/fixtures.py index 4504f72..ec4d412 100644 --- a/modflow_devtools/fixtures.py +++ b/modflow_devtools/fixtures.py @@ -17,11 +17,7 @@ @pytest.fixture(scope="function") def function_tmpdir(tmpdir_factory, request) -> Path: - node = ( - request.node.name.replace("/", "_") - .replace("\\", "_") - .replace(":", "_") - ) + node = request.node.name.replace("/", "_").replace("\\", "_").replace(":", "_") temp = Path(tmpdir_factory.mktemp(node)) yield Path(temp) @@ -269,9 +265,7 @@ def get_repo_path(repo_name: str) -> Optional[Path]: if repo_path else [] ) - metafunc.parametrize( - key, namefile_paths, ids=[str(m) for m in namefile_paths] - ) + metafunc.parametrize(key, namefile_paths, ids=[str(m) for m in namefile_paths]) key = "test_model_mf5to6" if key in metafunc.fixturenames: @@ -288,9 +282,7 @@ def get_repo_path(repo_name: str) -> Optional[Path]: if repo_path else [] ) - metafunc.parametrize( - key, namefile_paths, ids=[str(m) for m in namefile_paths] - ) + metafunc.parametrize(key, namefile_paths, ids=[str(m) for m in namefile_paths]) key = "large_test_model" if key in metafunc.fixturenames: @@ -307,9 +299,7 @@ def get_repo_path(repo_name: str) -> Optional[Path]: if repo_path else [] ) - metafunc.parametrize( - key, namefile_paths, ids=[str(m) for m in namefile_paths] - ) + metafunc.parametrize(key, namefile_paths, ids=[str(m) for m in namefile_paths]) key = "example_scenario" if key in metafunc.fixturenames: @@ -384,9 +374,7 @@ def get_examples(): filtered.append(name) break examples = { - name: nfps - for name, nfps in examples.items() - if name in filtered + name: nfps for name, nfps in examples.items() if name in filtered } # exclude mf6gwf and mf6gwt subdirs diff --git a/modflow_devtools/imports.py b/modflow_devtools/imports.py index 2521fcc..625c59d 100644 --- a/modflow_devtools/imports.py +++ b/modflow_devtools/imports.py @@ -130,9 +130,7 @@ def import_optional_dependency( module_to_get = sys.modules[install_name] else: module_to_get = module - minimum_version = ( - min_version if min_version is not None else VERSIONS.get(parent) - ) + minimum_version = min_version if min_version is not None else VERSIONS.get(parent) if minimum_version: version = get_version(module_to_get) if Version(version) < Version(minimum_version): diff --git a/modflow_devtools/latex.py b/modflow_devtools/latex.py index bbe766c..4670c2d 100644 --- a/modflow_devtools/latex.py +++ b/modflow_devtools/latex.py @@ -92,11 +92,7 @@ def get_header( header = "\\small\n" header += "\\begin{longtable}[!htbp]{\n" for col_width in col_widths: - header += ( - 38 * " " - + f"{align}" - + f"{{{col_width}\\linewidth-2\\arraycolsep}}\n" - ) + header += 38 * " " + f"{align}" + f"{{{col_width}\\linewidth-2\\arraycolsep}}\n" header += 38 * " " + "}\n" header += f"\t\\caption{{{caption}}} \\label{{{label}}} \\\\\n\n" diff --git a/modflow_devtools/markers.py b/modflow_devtools/markers.py index 1c21e43..7bfeaf3 100644 --- a/modflow_devtools/markers.py +++ b/modflow_devtools/markers.py @@ -31,7 +31,7 @@ def requires_exe(*exes): def requires_python(version, bound="lower"): if not isinstance(version, str): - raise ValueError(f"Version must a string") + raise ValueError("Version must a string") py_tgt = Version(version) if bound == "lower": @@ -57,25 +57,21 @@ def requires_pkg(*pkgs): def requires_platform(platform, ci_only=False): return pytest.mark.skipif( - system().lower() != platform.lower() - and (is_in_ci() if ci_only else True), + system().lower() != platform.lower() and (is_in_ci() if ci_only else True), reason=f"only compatible with platform: {platform.lower()}", ) def excludes_platform(platform, ci_only=False): return pytest.mark.skipif( - system().lower() == platform.lower() - and (is_in_ci() if ci_only else True), + system().lower() == platform.lower() and (is_in_ci() if ci_only else True), reason=f"not compatible with platform: {platform.lower()}", ) def requires_branch(branch): current = get_current_branch() - return pytest.mark.skipif( - current != branch, reason=f"must run on branch: {branch}" - ) + return pytest.mark.skipif(current != branch, reason=f"must run on branch: {branch}") def excludes_branch(branch): diff --git a/modflow_devtools/misc.py b/modflow_devtools/misc.py index c7fb0de..e5ce467 100644 --- a/modflow_devtools/misc.py +++ b/modflow_devtools/misc.py @@ -16,6 +16,7 @@ from urllib import request from _warnings import warn +from urllib.error import URLError @contextmanager @@ -167,8 +168,8 @@ def get_packages(namefile_path: PathLike) -> List[str]: packages = [] path = Path(namefile_path).expanduser().absolute() lines = open(path, "r").readlines() - gwf_lines = [l for l in lines if l.strip().lower().startswith("gwf6 ")] - gwt_lines = [l for l in lines if l.strip().lower().startswith("gwt6 ")] + gwf_lines = [ln for ln in lines if ln.strip().lower().startswith("gwf6 ")] + gwt_lines = [ln for ln in lines if ln.strip().lower().startswith("gwt6 ")] def parse_model_namefile(line): nf_path = [path.parent / s for s in line.split(" ") if s != ""][1] @@ -181,24 +182,20 @@ def parse_model_namefile(line): # load model namefiles try: for line in gwf_lines: - packages = ( - packages + get_packages(parse_model_namefile(line)) + ["gwf"] - ) + packages = packages + get_packages(parse_model_namefile(line)) + ["gwf"] for line in gwt_lines: - packages = ( - packages + get_packages(parse_model_namefile(line)) + ["gwt"] - ) - except: + packages = packages + get_packages(parse_model_namefile(line)) + ["gwt"] + except: # noqa: E722 warn(f"Invalid namefile format: {traceback.format_exc()}") for line in lines: # Skip over blank and commented lines - ll = line.strip().split() - if len(ll) < 2: + line = line.strip().split() + if len(line) < 2: continue - l = ll[0].lower() - if any(l.startswith(c) for c in ["#", "!", "data", "list"]) or l in [ + line = line[0].lower() + if any(line.startswith(c) for c in ["#", "!", "data", "list"]) or line in [ "begin", "end", "memory_print_option", @@ -206,9 +203,9 @@ def parse_model_namefile(line): continue # strip "6" from package name - l = l.replace("6", "") + line = line.replace("6", "") - packages.append(l.lower()) + packages.append(line.lower()) return list(set(packages)) @@ -242,17 +239,12 @@ def get_namefile_paths( # find simulation namefiles paths = [ - p - for p in Path(path).rglob( - f"{prefix}*/**/{namefile}" if prefix else namefile - ) + p for p in Path(path).rglob(f"{prefix}*/**/{namefile}" if prefix else namefile) ] # remove excluded paths = [ - p - for p in paths - if (not excluded or not any(e in str(p) for e in excluded)) + p for p in paths if (not excluded or not any(e in str(p) for e in excluded)) ] # filter by package @@ -260,9 +252,7 @@ def get_namefile_paths( filtered = [] for nfp in paths: nf_pkgs = get_packages(nfp) - shared = set(nf_pkgs).intersection( - set([p.lower() for p in packages]) - ) + shared = set(nf_pkgs).intersection(set([p.lower() for p in packages])) if any(shared): filtered.append(nfp) paths = filtered @@ -271,9 +261,7 @@ def get_namefile_paths( if selected: paths = [ namfile_path - for (namfile_path, model_path) in zip( - paths, [p.parent for p in paths] - ) + for (namfile_path, model_path) in zip(paths, [p.parent for p in paths]) if any(s in model_path.name for s in selected) ] @@ -298,9 +286,7 @@ def get_model_paths( namefile_paths = get_namefile_paths( path, prefix, namefile, excluded, selected, packages ) - model_paths = sorted( - list(set([p.parent for p in namefile_paths if p.parent.name])) - ) + model_paths = sorted(list(set([p.parent for p in namefile_paths if p.parent.name]))) return model_paths @@ -341,16 +327,14 @@ def is_github_rate_limited() -> Optional[bool]: True if rate-limiting is applied, otherwise False (or None if the connection fails). """ try: - with request.urlopen( - "https://api.github.com/users/octocat" - ) as response: + with request.urlopen("https://api.github.com/users/octocat") as response: remaining = int(response.headers["x-ratelimit-remaining"]) if remaining < 10: warn( f"Only {remaining} GitHub API requests remaining before rate-limiting" ) return remaining > 0 - except: + except (ValueError, URLError): return None @@ -481,7 +465,7 @@ def get_env(name: str, default: object = None) -> Optional[object]: if isinstance(default, bool): v = v.lower().title() v = literal_eval(v) - except: + except (AttributeError, ValueError, TypeError, SyntaxError, MemoryError, RecursionError): return default if default is None: return v diff --git a/modflow_devtools/ostags.py b/modflow_devtools/ostags.py index 653f533..14186fc 100644 --- a/modflow_devtools/ostags.py +++ b/modflow_devtools/ostags.py @@ -73,10 +73,10 @@ def _suffixes(tag): try: return _suffixes(ostag.lower()) - except: + except KeyError: try: return _suffixes(python_to_modflow_ostag(ostag)) - except: + except KeyError: return _suffixes(github_to_modflow_ostag(ostag)) diff --git a/pyproject.toml b/pyproject.toml index 16c2018..bb40cd3 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -44,11 +44,7 @@ dynamic = ["version"] [project.optional-dependencies] lint = [ - "black", - "cffconvert", - "flake8", - "isort", - "pylint" + "ruff" ] test = [ "modflow-devtools[lint]", @@ -75,19 +71,19 @@ docs = [ "Bug Tracker" = "https://github.com/MODFLOW-USGS/modflow-devtools/issues" "Source Code" = "https://github.com/MODFLOW-USGS/modflow-devtools" +[tool.ruff] +target-version = "py38" +include = [ + "pyproject.toml", + "modflow_devtools/**/*.py", + "autotest/**/*.py", + "docs/**/*.py", + "scripts/**/*.py", + ".github/**/*.py", +] -[tool.black] -line-length = 79 -target_version = ["py37"] - -[tool.flynt] -line-length = 79 -verbose = true - -[tool.isort] -profile = "black" -src_paths = ["modflow_devtools"] -line_length = 79 +[tool.ruff.lint] +select = ["NPY201"] [tool.setuptools] packages = ["modflow_devtools"] diff --git a/scripts/update_version.py b/scripts/update_version.py index c79e5bf..92a7724 100644 --- a/scripts/update_version.py +++ b/scripts/update_version.py @@ -1,10 +1,7 @@ import argparse import textwrap from datetime import datetime -from enum import Enum -from os import PathLike from pathlib import Path -from typing import NamedTuple from filelock import FileLock from packaging.version import Version @@ -13,9 +10,7 @@ _project_root_path = Path(__file__).parent.parent _version_txt_path = _project_root_path / "version.txt" _package_init_path = _project_root_path / "modflow_devtools" / "__init__.py" -_readme_path = _project_root_path / "README.md" _docs_config_path = _project_root_path / "docs" / "conf.py" -_initial_version = Version("0.0.1") _current_version = Version(_version_txt_path.read_text().strip()) @@ -37,7 +32,7 @@ def update_init_py(timestamp: datetime, version: Version): print(f"Updated {_package_init_path} to version {version}") -def update_docs_config(timestamp: datetime, version: Version): +def update_docs_config(version: Version): lines = _docs_config_path.read_text().rstrip().split("\n") with open(_docs_config_path, "w") as f: for line in lines: @@ -52,8 +47,8 @@ def update_version( version: Version = None, ): lock_path = Path(_version_txt_path.name + ".lock") - try: - lock = FileLock(lock_path) + lock = FileLock(lock_path) + with lock: previous = Version(_version_txt_path.read_text().strip()) version = ( version @@ -61,15 +56,9 @@ def update_version( else Version(previous.major, previous.minor, previous.micro) ) - with lock: - update_version_txt(version) - update_init_py(timestamp, version) - update_docs_config(timestamp, version) - finally: - try: - lock_path.unlink() - except: - pass + update_version_txt(version) + update_init_py(timestamp, version) + update_docs_config(version) if __name__ == "__main__": @@ -106,7 +95,5 @@ def update_version( else: update_version( timestamp=datetime.now(), - version=( - Version(args.version) if args.version else _current_version - ), + version=(Version(args.version) if args.version else _current_version), ) From e65c4d3304f0230345d6f1d4d8f911e0c2eb7c64 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Mon, 11 Mar 2024 16:46:38 -0400 Subject: [PATCH 06/14] chore: don't specify ruff rules explicitly (#146) --- pyproject.toml | 3 --- 1 file changed, 3 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index bb40cd3..794e61c 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,9 +82,6 @@ include = [ ".github/**/*.py", ] -[tool.ruff.lint] -select = ["NPY201"] - [tool.setuptools] packages = ["modflow_devtools"] include-package-data = true From 08eff72b73da66541f7a08ddf398298210ce3758 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Thu, 4 Apr 2024 13:19:26 -0400 Subject: [PATCH 07/14] chore(format): add isort rule, enforce line length (#147) --- autotest/test_download.py | 3 ++- autotest/test_markers.py | 14 +++++++------- autotest/test_misc.py | 3 ++- autotest/test_zip.py | 2 +- modflow_devtools/download.py | 9 ++++++--- modflow_devtools/fixtures.py | 27 ++++++++++++++++++--------- modflow_devtools/imports.py | 2 +- modflow_devtools/misc.py | 31 ++++++++++++++++++++++--------- pyproject.toml | 3 +++ scripts/update_version.py | 3 ++- 10 files changed, 64 insertions(+), 33 deletions(-) diff --git a/autotest/test_download.py b/autotest/test_download.py index dc3b43b..b1ecc47 100644 --- a/autotest/test_download.py +++ b/autotest/test_download.py @@ -51,7 +51,8 @@ def test_get_release(repo): actual_names = [asset["name"] for asset in assets] if repo == "MODFLOW-USGS/modflow6": - # can remove if modflow6 releases follow asset name conventions followed in executables and nightly build repos + # can remove if modflow6 releases follow asset name + # conventions followed in executables and nightly build repos assert set([a.rpartition("_")[2] for a in actual_names]) >= set( [a for a in expected_names if not a.startswith("win")] ) diff --git a/autotest/test_markers.py b/autotest/test_markers.py index e93020b..8df2a49 100644 --- a/autotest/test_markers.py +++ b/autotest/test_markers.py @@ -2,21 +2,21 @@ from platform import python_version, system from shutil import which -from packaging.version import Version import pytest +from packaging.version import Version from modflow_devtools.markers import ( - requires_exe, + excludes_platform, require_exe, - requires_program, + require_package, + require_platform, require_program, + require_python, + requires_exe, requires_pkg, - require_package, requires_platform, - require_platform, - excludes_platform, + requires_program, requires_python, - require_python, ) exe = "pytest" diff --git a/autotest/test_misc.py b/autotest/test_misc.py index e6c5b00..c3034a3 100644 --- a/autotest/test_misc.py +++ b/autotest/test_misc.py @@ -93,7 +93,8 @@ def test_get_packages_fails_on_invalid_namefile(module_tmpdir): namefile_path = new_model_path / "mfsim.nam" shutil.copytree(model_path, new_model_path) - # invalid gwf namefile reference - result should only contain packages from mfsim.nam + # invalid gwf namefile reference: + # result should only contain packages from mfsim.nam lines = open(namefile_path, "r").read().splitlines() with open(namefile_path, "w") as f: for line in lines: diff --git a/autotest/test_zip.py b/autotest/test_zip.py index 070b9c4..96f1c79 100644 --- a/autotest/test_zip.py +++ b/autotest/test_zip.py @@ -40,7 +40,7 @@ def test_compressall(function_tmpdir): @pytest.fixture(scope="module") def empty_archive(module_tmpdir) -> Path: # https://stackoverflow.com/a/25195628/6514033 - data = b"PK\x05\x06\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" + data = b"PK\x05\x06\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00" # noqa: E501 path = module_tmpdir / "empty.zip" with open(path, "wb") as zip: zip.write(data) diff --git a/modflow_devtools/download.py b/modflow_devtools/download.py index 719220d..4c2b67e 100644 --- a/modflow_devtools/download.py +++ b/modflow_devtools/download.py @@ -81,7 +81,10 @@ def get_response_json(): try: if verbose: print( - f"Fetching releases for repo {repo} (page {page}, {per_page} per page)" + f"Fetching releases for " + f"repo {repo} " + f"(page {page}, " + f"{per_page} per page)" ) with urllib.request.urlopen(request, timeout=10) as resp: return json.loads(resp.read().decode()) @@ -285,8 +288,8 @@ def list_artifacts( Returns ------- - A list of dictionaries, each containing information about an artifact as returned - by the GitHub API. + A list of dictionaries, each containing information + about an artifact as returned by the GitHub API. """ if "/" not in repo: diff --git a/modflow_devtools/fixtures.py b/modflow_devtools/fixtures.py index ec4d412..fe7b1ce 100644 --- a/modflow_devtools/fixtures.py +++ b/modflow_devtools/fixtures.py @@ -115,9 +115,12 @@ def pytest_addoption(parser): action="store", default=None, dest="KEEP", - help="Move the contents of temporary test directories to correspondingly named subdirectories at the given " - "location after tests complete. This option can be used to exclude test results from automatic cleanup, " - "e.g. for manual inspection. The provided path is created if it does not already exist. An error is " + help="Move the contents of temporary test directories to " + "correspondingly named subdirectories at the given " + "location after tests complete. This option can be used " + "to exclude test results from automatic cleanup, " + "e.g. for manual inspection. The provided path is " + "created if it does not already exist. An error is " "thrown if any matching files already exist.", ) @@ -125,9 +128,12 @@ def pytest_addoption(parser): "--keep-failed", action="store", default=None, - help="Move the contents of temporary test directories to correspondingly named subdirectories at the given " - "location if the test case fails. This option automatically saves the outputs of failed tests in the " - "given location. The path is created if it doesn't already exist. An error is thrown if files with the " + help="Move the contents of temporary test directories to " + "correspondingly named subdirectories at the given " + "location if the test case fails. This option saves " + "the outputs of failed tests in the " + "given location. The path is created if it doesn't " + "already exist. An error is thrown if files with the " "same names already exist in the given location.", ) @@ -144,7 +150,8 @@ def pytest_addoption(parser): "--meta", action="store", metavar="NAME", - help="Indicates a test should only be run by other tests (e.g., to test framework or fixtures).", + help="Indicates a test should only be run by other tests (e.g., " + "to test framework or fixtures).", ) parser.addoption( @@ -167,7 +174,8 @@ def pytest_addoption(parser): action="store", default="yes", dest="PANDAS", - help="Indicates whether to use pandas, where multiple approaches are available. Select 'yes', 'no', or 'random'.", + help="Indicates whether to use pandas, where multiple approaches " + "are available. Select 'yes', 'no', or 'random'.", ) parser.addoption( @@ -176,7 +184,8 @@ def pytest_addoption(parser): action="store", default="raw", dest="TABULAR", - help="Configure tabular data representation for model input. Select 'raw', 'recarray', or 'dataframe'.", + help="Configure tabular data representation for model input. " + "Select 'raw', 'recarray', or 'dataframe'.", ) diff --git a/modflow_devtools/imports.py b/modflow_devtools/imports.py index 625c59d..909e1d2 100644 --- a/modflow_devtools/imports.py +++ b/modflow_devtools/imports.py @@ -3,7 +3,7 @@ # This file is dual licensed under the terms of the BSD 3-Clause License. # BSD 3-Clause License # -# Copyright (c) 2008-2011, AQR Capital Management, LLC, Lambda Foundry, Inc. and PyData Development Team +# Copyright (c) 2008-2011, AQR Capital Management, LLC, Lambda Foundry, Inc. and PyData Development Team # noqa: E501 # All rights reserved. # # Copyright (c) 2011-2021, Open source contributors. diff --git a/modflow_devtools/misc.py b/modflow_devtools/misc.py index e5ce467..fac9ce9 100644 --- a/modflow_devtools/misc.py +++ b/modflow_devtools/misc.py @@ -14,9 +14,9 @@ from timeit import timeit from typing import List, Optional, Tuple from urllib import request +from urllib.error import URLError from _warnings import warn -from urllib.error import URLError @contextmanager @@ -71,7 +71,8 @@ def get_ostag() -> str: def get_suffixes(ostag) -> Tuple[str, str]: """ - Returns executable and library suffixes for the given OS (as returned by sys.platform) + Returns executable and library suffixes for the + given OS (as returned by sys.platform) .. deprecated:: 1.1.0 Use ``ostags.get_binary_suffixes()`` instead. @@ -151,9 +152,11 @@ def get_current_branch() -> str: def get_packages(namefile_path: PathLike) -> List[str]: """ - Return a list of packages used by the simulation or model defined in the given namefile. - The namefile may be for an entire simulation or for a GWF or GWT model. If a simulation - namefile is given, packages used in its component model namefiles will be included. + Return a list of packages used by the simulation + or model defined in the given namefile. The namefile + may be for an entire simulation or for a GWF or GWT + model. If a simulation namefile is given, packages + used in its component model namefiles will be included. Parameters ---------- @@ -175,7 +178,8 @@ def parse_model_namefile(line): nf_path = [path.parent / s for s in line.split(" ") if s != ""][1] if nf_path.suffix != ".nam": raise ValueError( - f"Failed to parse GWF or GWT model namefile from simulation namefile line: {line}" + "Failed to parse GWF or GWT model namefile " + f"from simulation namefile line: {line}" ) return nf_path @@ -324,14 +328,16 @@ def is_github_rate_limited() -> Optional[bool]: Returns ------- - True if rate-limiting is applied, otherwise False (or None if the connection fails). + True if rate-limiting is applied, otherwise False + (or None if the connection fails). """ try: with request.urlopen("https://api.github.com/users/octocat") as response: remaining = int(response.headers["x-ratelimit-remaining"]) if remaining < 10: warn( - f"Only {remaining} GitHub API requests remaining before rate-limiting" + f"Only {remaining} GitHub API requests " + "remaining before rate-limiting" ) return remaining > 0 except (ValueError, URLError): @@ -465,7 +471,14 @@ def get_env(name: str, default: object = None) -> Optional[object]: if isinstance(default, bool): v = v.lower().title() v = literal_eval(v) - except (AttributeError, ValueError, TypeError, SyntaxError, MemoryError, RecursionError): + except ( + AttributeError, + ValueError, + TypeError, + SyntaxError, + MemoryError, + RecursionError, + ): return default if default is None: return v diff --git a/pyproject.toml b/pyproject.toml index 794e61c..2fd9627 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -82,6 +82,9 @@ include = [ ".github/**/*.py", ] +[tool.ruff.lint] +select = ["F", "E", "I001"] + [tool.setuptools] packages = ["modflow_devtools"] include-package-data = true diff --git a/scripts/update_version.py b/scripts/update_version.py index 92a7724..26d34c0 100644 --- a/scripts/update_version.py +++ b/scripts/update_version.py @@ -86,7 +86,8 @@ def update_version( "--get", required=False, action="store_true", - help="Just get the current version number, don't update anything (defaults to false)", + help="Just get the current version number, " + "don't update anything (defaults to false)", ) args = parser.parse_args() From 1f358de2bc721c1000c3d0823b9440776432e3b0 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Fri, 12 Apr 2024 10:21:20 -0400 Subject: [PATCH 08/14] feat(markers): add no_parallel marker, support differing pkg/module names (#148) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * add no_parallel marker to modflow_devtools.markers to skip test if xdist is activated * add name_map param to has_pkg() function and requires_pkg marker — accommodate packages which don't have a correspondingly named top-level module, e.g. pytext-xdist -> xdist, mfpymake -> pymake * minor refactor in has_pkg() and update docstrings --- autotest/test_markers.py | 16 +++++++++++++++ docs/md/markers.md | 12 ++++++++++++ modflow_devtools/markers.py | 11 +++++++++-- modflow_devtools/misc.py | 39 +++++++++++++++++++++++++------------ 4 files changed, 64 insertions(+), 14 deletions(-) diff --git a/autotest/test_markers.py b/autotest/test_markers.py index 8df2a49..4e1718c 100644 --- a/autotest/test_markers.py +++ b/autotest/test_markers.py @@ -7,6 +7,7 @@ from modflow_devtools.markers import ( excludes_platform, + no_parallel, require_exe, require_package, require_platform, @@ -75,3 +76,18 @@ def test_requires_python(version): if Version(py_ver) >= Version(version): assert requires_python(version) assert require_python(version) + + +@no_parallel +@requires_pkg("pytest-xdist", name_map={"pytest-xdist": "xdist"}) +def test_no_parallel(worker_id): + """ + Should only run with xdist disabled, in which case: + - xdist environment variables are not set + - worker_id is 'master' (assuming xdist is installed) + + See https://pytest-xdist.readthedocs.io/en/stable/how-to.html#identifying-the-worker-process-during-a-test. + """ + + assert environ.get("PYTEST_XDIST_WORKER") is None + assert worker_id == "master" diff --git a/docs/md/markers.md b/docs/md/markers.md index a50aec3..aa963c1 100644 --- a/docs/md/markers.md +++ b/docs/md/markers.md @@ -80,6 +80,18 @@ Markers are also provided to ping network resources and skip if unavailable: - `@requires_github`: skips if `github.com` is unreachable - `@requires_spatial_reference`: skips if `spatialreference.org` is unreachable +A marker is also available to skip tests if `pytest` is running in parallel with [`pytest-xdist`](https://pytest-xdist.readthedocs.io/en/latest/): + +```python +from os import environ +from modflow_devtools.markers import no_parallel + +@no_parallel +def test_only_serially(): + # https://pytest-xdist.readthedocs.io/en/stable/how-to.html#identifying-the-worker-process-during-a-test. + assert environ.get("PYTEST_XDIST_WORKER") is None +``` + ## Aliases All markers are aliased to imperative mood, e.g. `require_github`. Some have other aliases as well: diff --git a/modflow_devtools/markers.py b/modflow_devtools/markers.py index 7bfeaf3..c87fcbe 100644 --- a/modflow_devtools/markers.py +++ b/modflow_devtools/markers.py @@ -3,7 +3,9 @@ Occasionally useful to directly assert environment expectations. """ +from os import environ from platform import python_version, system +from typing import Dict, Optional from packaging.version import Version @@ -46,8 +48,8 @@ def requires_python(version, bound="lower"): ) -def requires_pkg(*pkgs): - missing = {pkg for pkg in pkgs if not has_pkg(pkg, strict=True)} +def requires_pkg(*pkgs, name_map: Optional[Dict[str, str]] = None): + missing = {pkg for pkg in pkgs if not has_pkg(pkg, strict=True, name_map=name_map)} return pytest.mark.skipif( missing, reason=f"missing package{'s' if len(missing) != 1 else ''}: " @@ -81,6 +83,11 @@ def excludes_branch(branch): ) +no_parallel = pytest.mark.skipif( + environ.get("PYTEST_XDIST_WORKER_COUNT"), reason="can't run in parallel" +) + + requires_github = pytest.mark.skipif( not is_connected("github.com"), reason="github.com is required." ) diff --git a/modflow_devtools/misc.py b/modflow_devtools/misc.py index fac9ce9..19931ee 100644 --- a/modflow_devtools/misc.py +++ b/modflow_devtools/misc.py @@ -12,7 +12,7 @@ from shutil import which from subprocess import PIPE, Popen from timeit import timeit -from typing import List, Optional, Tuple +from typing import Dict, List, Optional, Tuple from urllib import request from urllib.error import URLError @@ -359,7 +359,9 @@ def has_exe(exe): return _has_exe_cache[exe] -def has_pkg(pkg: str, strict: bool = False) -> bool: +def has_pkg( + pkg: str, strict: bool = False, name_map: Optional[Dict[str, str]] = None +) -> bool: """ Determines if the given Python package is installed. @@ -368,8 +370,13 @@ def has_pkg(pkg: str, strict: bool = False) -> bool: pkg : str Name of the package to check. strict : bool - If False, only check if package metadata is available. + If False, only check if the package is cached or metadata is available. If True, try to import the package (all dependencies must be present). + name_map : dict, optional + Custom mapping between package names (as provided to `metadata.distribution`) + and module names (as used in import statements or `importlib.import_module`). + Useful for packages whose package names do not match the module name, e.g. + `pytest-xdist` and `xdist`, respectively, or `mfpymake` and `pymake`. Returns ------- @@ -378,12 +385,19 @@ def has_pkg(pkg: str, strict: bool = False) -> bool: Notes ----- + If `strict=True` and a package name differs from its top-level module name, a + `name_map` must be provided, otherwise this function will return False even if + the package is installed. + Originally written by Mike Toews (mwtoews@gmail.com) for FloPy. """ - def try_import(): + def get_module_name() -> str: + return pkg if name_map is None else name_map.get(pkg, pkg) + + def try_import() -> bool: try: # import name, e.g. "import shapefile" - importlib.import_module(pkg) + importlib.import_module(get_module_name()) return True except ModuleNotFoundError: return False @@ -395,14 +409,15 @@ def try_metadata() -> bool: except metadata.PackageNotFoundError: return False - found = False - if not strict: - found = pkg in _has_pkg_cache or try_metadata() - if not found: - found = try_import() + is_cached = pkg in _has_pkg_cache + has_metadata = try_metadata() + can_import = try_import() + if strict: + found = has_metadata and can_import + else: + found = has_metadata or is_cached _has_pkg_cache[pkg] = found - - return _has_pkg_cache[pkg] + return found def timed(f): From a59b426f06f6a48fd712bbb7c3beed8f54c69d33 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Fri, 12 Apr 2024 12:38:35 -0400 Subject: [PATCH 09/14] ci(rtd): trigger rtd build if ci tests succeed (#149) --- .github/workflows/ci.yml | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e9fea5f..3c34589 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -170,4 +170,17 @@ jobs: env: REPOS_PATH: ${{ github.workspace }} GITHUB_TOKEN: ${{ github.token }} - run: pytest -v -n auto --durations 0 test_download.py \ No newline at end of file + run: pytest -v -n auto --durations 0 test_download.py + + rtd: + name: Docs + needs: test + runs-on: ubuntu-22.04 + if: github.repository_owner == 'MODFLOW-USGS' && github.event_name == 'push' + steps: + - name: Trigger RTD + uses: dfm/rtds-action@v1 + with: + webhook_url: ${{ secrets.RTDS_WEBHOOK_URL }} + webhook_token: ${{ secrets.RTDS_WEBHOOK_TOKEN }} + commit_ref: ${{ github.ref }} From 748c935610c53fe90be83a6f580dc74a08f7c4df Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Fri, 12 Apr 2024 13:01:35 -0400 Subject: [PATCH 10/14] docs(markers): fix aliases section (#150) --- docs/md/markers.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/md/markers.md b/docs/md/markers.md index aa963c1..8c9460e 100644 --- a/docs/md/markers.md +++ b/docs/md/markers.md @@ -96,5 +96,5 @@ def test_only_serially(): All markers are aliased to imperative mood, e.g. `require_github`. Some have other aliases as well: -`requires_pkg` -> `require[s]_package` -`requires_exe` -> `require[s]_program` +- `requires_pkg` -> `require[s]_package` +- `requires_exe` -> `require[s]_program` From c9e445dd1544413f3729c7a78c2a77038db80050 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Mon, 13 May 2024 13:11:10 -0400 Subject: [PATCH 11/14] feat(snapshots): add snapshot fixtures, remove pandas fixture (#151) * add numpy array snapshot extensions and fixtures with syrupy, moved from flopy * remove use_pandas fixture, originally meant for use in flopy tests for pandas support but never used and probably not needed, as flopy use_pandas flags will be removed eventually * todo: figure out how to support multi-array snapshots --- README.md | 3 +- .../test_binary_array_snapshot.npy | Bin 0 -> 152 bytes .../test_readable_text_array_snapshot.txt | 1 + .../test_text_array_snapshot.txt | 3 + autotest/test_fixtures.py | 102 ++++++++----- docs/md/fixtures.md | 10 ++ modflow_devtools/fixtures.py | 138 ++++++++++++++---- pyproject.toml | 4 +- 8 files changed, 191 insertions(+), 70 deletions(-) create mode 100644 autotest/__snapshots__/test_fixtures/test_binary_array_snapshot.npy create mode 100644 autotest/__snapshots__/test_fixtures/test_readable_text_array_snapshot.txt create mode 100644 autotest/__snapshots__/test_fixtures/test_text_array_snapshot.txt diff --git a/README.md b/README.md index 688cd9e..b81da4a 100644 --- a/README.md +++ b/README.md @@ -56,6 +56,7 @@ Python3.8+, dependency-free, but pairs well with `pytest` and select plugins, e. - [`pytest-dotenv`](https://github.com/quiqua/pytest-dotenv) - [`pytest-xdist`](https://github.com/pytest-dev/pytest-xdist) +- [`syrupy`](https://github.com/tophat/syrupy) ## Installation @@ -65,7 +66,7 @@ Python3.8+, dependency-free, but pairs well with `pytest` and select plugins, e. pip install modflow-devtools ``` -Pytest, pytest plugins, and other optional dependencies can be installed with: +Pytest, pytest plugins, and other testing-related dependencies can be installed with: ```shell pip install "modflow-devtools[test]" diff --git a/autotest/__snapshots__/test_fixtures/test_binary_array_snapshot.npy b/autotest/__snapshots__/test_fixtures/test_binary_array_snapshot.npy new file mode 100644 index 0000000000000000000000000000000000000000..0e7fc14e59e069e932be5b7fa14d37896493396a GIT binary patch literal 152 zcmbR27wQ`j$;eQ~P_3SlTAW;@Zl$1ZlV+i=qoAIaUsO_*m=~X4l#&V(cT3DEP6dh= jXCxM+0{I%oI+{8PwF(pfu30ld;G;c+W^_mc11<*uhzKUD literal 0 HcmV?d00001 diff --git a/autotest/__snapshots__/test_fixtures/test_readable_text_array_snapshot.txt b/autotest/__snapshots__/test_fixtures/test_readable_text_array_snapshot.txt new file mode 100644 index 0000000..1bc3865 --- /dev/null +++ b/autotest/__snapshots__/test_fixtures/test_readable_text_array_snapshot.txt @@ -0,0 +1 @@ +[1.1 2.2 3.3] \ No newline at end of file diff --git a/autotest/__snapshots__/test_fixtures/test_text_array_snapshot.txt b/autotest/__snapshots__/test_fixtures/test_text_array_snapshot.txt new file mode 100644 index 0000000..c747aba --- /dev/null +++ b/autotest/__snapshots__/test_fixtures/test_text_array_snapshot.txt @@ -0,0 +1,3 @@ +1.100000000000000089e+00 +2.200000000000000178e+00 +3.299999999999999822e+00 diff --git a/autotest/test_fixtures.py b/autotest/test_fixtures.py index efddf54..600a34c 100644 --- a/autotest/test_fixtures.py +++ b/autotest/test_fixtures.py @@ -2,11 +2,13 @@ import platform from pathlib import Path +import numpy as np import pytest from _pytest.config import ExitCode system = platform.system() -proj_root = Path(__file__).parent.parent.parent.parent +proj_root = Path(__file__).parents[1] +module_path = Path(inspect.getmodulename(__file__)) # test temporary directory fixtures @@ -63,7 +65,7 @@ def test_class_scoped_tmpdir(self, class_tmpdir): def test_module_scoped_tmpdir(module_tmpdir): assert isinstance(module_tmpdir, Path) assert module_tmpdir.is_dir() - assert Path(inspect.getmodulename(__file__)).stem in module_tmpdir.name + assert module_path.stem in module_tmpdir.name def test_session_scoped_tmpdir(session_tmpdir): @@ -269,41 +271,7 @@ def test_large_test_model(large_test_model): assert large_test_model.name == "mfsim.nam" -# test pandas fixture - -test_pandas_fname = "pandas.txt" - - -@pytest.mark.meta("test_pandas") -def test_pandas_inner(function_tmpdir, use_pandas): - with open(function_tmpdir / test_pandas_fname, "w") as f: - f.write(str(use_pandas)) - - -@pytest.mark.parametrize("pandas", ["yes", "no", "random"]) -@pytest.mark.parametrize("arg", ["--pandas", "-P"]) -def test_pandas(pandas, arg, function_tmpdir): - inner_fn = test_pandas_inner.__name__ - args = [ - __file__, - "-v", - "-s", - "-k", - inner_fn, - arg, - pandas, - "--keep", - function_tmpdir, - "-M", - "test_pandas", - ] - assert pytest.main(args) == ExitCode.OK - res = open(next(function_tmpdir.rglob(test_pandas_fname))).readlines()[0] - assert res - if pandas == "yes": - assert "True" in res - elif pandas == "no": - assert "False" in res +# test tabular data format fixture test_tabular_fname = "tabular.txt" @@ -335,3 +303,63 @@ def test_tabular(tabular, arg, function_tmpdir): assert pytest.main(args) == ExitCode.OK res = open(next(function_tmpdir.rglob(test_tabular_fname))).readlines()[0] assert tabular == res + + +# test snapshot fixtures + + +snapshot_array = np.array([1.1, 2.2, 3.3]) +snapshots_path = proj_root / "autotest" / "__snapshots__" + + +def test_binary_array_snapshot(array_snapshot): + assert array_snapshot == snapshot_array + snapshot_path = ( + snapshots_path + / module_path.stem + / f"{inspect.currentframe().f_code.co_name}.npy" + ) + assert snapshot_path.is_file() + assert np.allclose(np.load(snapshot_path), snapshot_array) + + +# todo: reinstate if/when we support multiple arrays +# def test_binary_array_snapshot_multi(array_snapshot): +# arrays = {"ascending": snapshot_array, "descending": np.flip(snapshot_array)} +# assert array_snapshot == arrays +# snapshot_path = ( +# snapshots_path +# / module_path.stem +# / f"{inspect.currentframe().f_code.co_name}.npy" +# ) +# assert snapshot_path.is_file() +# assert np.allclose(np.load(snapshot_path)["ascending"], snapshot_array) +# assert np.allclose(np.load(snapshot_path)["descending"], np.flip(snapshot_array)) + + +def test_text_array_snapshot(text_array_snapshot): + assert text_array_snapshot == snapshot_array + snapshot_path = ( + snapshots_path + / module_path.stem + / f"{inspect.currentframe().f_code.co_name}.txt" + ) + assert snapshot_path.is_file() + assert np.allclose(np.loadtxt(snapshot_path), snapshot_array) + + +def test_readable_text_array_snapshot(readable_array_snapshot): + assert readable_array_snapshot == snapshot_array + snapshot_path = ( + snapshots_path + / module_path.stem + / f"{inspect.currentframe().f_code.co_name}.txt" + ) + assert snapshot_path.is_file() + assert np.allclose( + np.fromstring( + open(snapshot_path).readlines()[0].replace("[", "").replace("]", ""), + sep=" ", + ), + snapshot_array, + ) diff --git a/docs/md/fixtures.md b/docs/md/fixtures.md index b847f10..613fd96 100644 --- a/docs/md/fixtures.md +++ b/docs/md/fixtures.md @@ -169,3 +169,13 @@ Model-loading fixtures use a set of utility functions to find and enumerate mode - `get_namefile_paths()` These functions are used internally in a `pytest_generate_tests` hook to implement the above model-parametrization fixtures. See `fixtures.py` and/or this project's test suite for usage examples. + +## Snapshot testing + +Snapshot testing is a form of regression testing in which a "snapshot" of the results of some computation is verified and captured by the developer to be compared against when tests are subsequently run. This is accomplished with [`syrupy`](https://github.com/tophat/syrupy), which provides a `snapshot` fixture overriding the equality operator to allow comparison with e.g. `snapshot == result`. A few custom fixtures for snapshots of NumPy arrays are also provided: + +- `array_snapshot`: saves an array in a binary file for compact storage, can be inspected programmatically with `np.load()` +- `text_array_snapshot`: flattens an array and stores it in a text file, compromise between readability and disk usage +- `readable_array_snapshot`: stores an array in a text file in its original shape, easy to inspect but largest on disk + +By default, tests run in comparison mode. This means a newly written test using any of the snapshot fixtures will fail until a snapshot is created. Snapshots can be created/updated by running pytest with the `--snapshot-update` flag. \ No newline at end of file diff --git a/modflow_devtools/fixtures.py b/modflow_devtools/fixtures.py index fe7b1ce..2d37328 100644 --- a/modflow_devtools/fixtures.py +++ b/modflow_devtools/fixtures.py @@ -1,22 +1,106 @@ -import random from collections import OrderedDict +from io import BytesIO, StringIO from itertools import groupby from os import PathLike, environ from pathlib import Path from shutil import copytree, rmtree -from typing import Dict, List, Optional +from typing import Dict, Generator, List, Optional from modflow_devtools.imports import import_optional_dependency from modflow_devtools.misc import get_namefile_paths, get_packages +np = import_optional_dependency("numpy") pytest = import_optional_dependency("pytest") +syrupy = import_optional_dependency("syrupy") + +# ruff: noqa: E402 +from syrupy.extensions.single_file import ( + SingleFileSnapshotExtension, + WriteMode, +) +from syrupy.types import ( + PropertyFilter, + PropertyMatcher, + SerializableData, + SerializedData, +) + +# snapshot extensions + + +class BinaryArrayExtension(SingleFileSnapshotExtension): + """ + Binary snapshot of a NumPy array. Can be read back into NumPy with + .load(), preserving dtype and shape. This is the recommended array + snapshot approach if human-readability is not a necessity, as disk + space is minimized. + """ + + _write_mode = WriteMode.BINARY + _file_extension = "npy" + + def serialize( + self, + data, + *, + exclude=None, + include=None, + matcher=None, + ): + buffer = BytesIO() + np.save(buffer, data) + return buffer.getvalue() + + +class TextArrayExtension(SingleFileSnapshotExtension): + """ + Text snapshot of a NumPy array. Flattens the array before writing. + Can be read back into NumPy with .loadtxt() assuming you know the + shape of the expected data and subsequently reshape it if needed. + """ + + _write_mode = WriteMode.TEXT + _file_extension = "txt" + + def serialize( + self, + data: "SerializableData", + *, + exclude: Optional["PropertyFilter"] = None, + include: Optional["PropertyFilter"] = None, + matcher: Optional["PropertyMatcher"] = None, + ) -> "SerializedData": + buffer = StringIO() + np.savetxt(buffer, data.ravel()) + return buffer.getvalue() + + +class ReadableArrayExtension(SingleFileSnapshotExtension): + """ + Human-readable snapshot of a NumPy array. Preserves array shape + at the expense of possible loss of precision (default 8 places) + and more difficulty loading into NumPy than TextArrayExtension. + """ + + _write_mode = WriteMode.TEXT + _file_extension = "txt" + + def serialize( + self, + data: "SerializableData", + *, + exclude: Optional["PropertyFilter"] = None, + include: Optional["PropertyFilter"] = None, + matcher: Optional["PropertyMatcher"] = None, + ) -> "SerializedData": + return np.array2string(data, threshold=np.inf) # fixtures @pytest.fixture(scope="function") -def function_tmpdir(tmpdir_factory, request) -> Path: +def function_tmpdir(tmpdir_factory, request) -> Generator[Path, None, None]: node = request.node.name.replace("/", "_").replace("\\", "_").replace(":", "_") temp = Path(tmpdir_factory.mktemp(node)) yield Path(temp) @@ -37,7 +121,7 @@ def function_tmpdir(tmpdir_factory, request) -> Path: @pytest.fixture(scope="class") -def class_tmpdir(tmpdir_factory, request) -> Path: +def class_tmpdir(tmpdir_factory, request) -> Generator[Path, None, None]: assert ( request.cls is not None ), "Class-scoped temp dir fixture must be used on class" @@ -53,7 +137,7 @@ def class_tmpdir(tmpdir_factory, request) -> Path: @pytest.fixture(scope="module") -def module_tmpdir(tmpdir_factory, request) -> Path: +def module_tmpdir(tmpdir_factory, request) -> Generator[Path, None, None]: temp = Path(tmpdir_factory.mktemp(request.module.__name__)) yield temp @@ -66,7 +150,7 @@ def module_tmpdir(tmpdir_factory, request) -> Path: @pytest.fixture(scope="session") -def session_tmpdir(tmpdir_factory, request) -> Path: +def session_tmpdir(tmpdir_factory, request) -> Generator[Path, None, None]: temp = Path(tmpdir_factory.mktemp(request.config.rootpath.name)) yield temp @@ -85,26 +169,28 @@ def repos_path() -> Optional[Path]: @pytest.fixture -def use_pandas(request): - pandas = request.config.option.PANDAS - if pandas == "yes": - return True - elif pandas == "no": - return False - elif pandas == "random": - return random.randint(0, 1) == 0 - else: - raise ValueError(f"Unsupported value for --pandas: {pandas}") - - -@pytest.fixture -def tabular(request): +def tabular(request) -> str: tab = request.config.option.TABULAR if tab not in ["raw", "recarray", "dataframe"]: raise ValueError(f"Unsupported value for --tabular: {tab}") return tab +@pytest.fixture +def array_snapshot(snapshot): + return snapshot.use_extension(BinaryArrayExtension) + + +@pytest.fixture +def text_array_snapshot(snapshot): + return snapshot.use_extension(TextArrayExtension) + + +@pytest.fixture +def readable_array_snapshot(snapshot): + return snapshot.use_extension(ReadableArrayExtension) + + # configuration hooks @@ -168,16 +254,6 @@ def pytest_addoption(parser): help="Select a subset of packages to run.", ) - parser.addoption( - "-P", - "--pandas", - action="store", - default="yes", - dest="PANDAS", - help="Indicates whether to use pandas, where multiple approaches " - "are available. Select 'yes', 'no', or 'random'.", - ) - parser.addoption( "-T", "--tabular", @@ -399,5 +475,5 @@ def get_examples(): metafunc.parametrize( key, [(name, nfps) for name, nfps in example_scenarios.items()], - ids=[name for name, ex in example_scenarios.items()], + ids=list(example_scenarios.keys()), ) diff --git a/pyproject.toml b/pyproject.toml index 2fd9627..ca2a2a5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -54,11 +54,13 @@ test = [ "meson!=0.63.0", "ninja", "numpy", + "pandas", "pytest!=8.1.0", "pytest-cov", "pytest-dotenv", "pytest-xdist", - "PyYaml" + "PyYaml", + "syrupy" ] docs = [ "sphinx", From d96089e512fbb79408e4fb58c89ee63da60dc727 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Mon, 13 May 2024 18:38:39 -0400 Subject: [PATCH 12/14] refactor(snapshots): move to separate module (#152) --- .github/workflows/release.yml | 1 - README.md | 2 +- .../test_binary_array_snapshot.npy | Bin .../test_readable_text_array_snapshot.txt | 0 .../test_text_array_snapshot.txt | 0 autotest/test_fixtures.py | 61 ---------- autotest/test_snapshots.py | 63 ++++++++++ docs/md/fixtures.md | 10 -- docs/md/snapshots.md | 17 +++ modflow_devtools/fixtures.py | 100 ---------------- modflow_devtools/snapshots.py | 108 ++++++++++++++++++ scripts/lint.py | 21 ---- 12 files changed, 189 insertions(+), 194 deletions(-) rename autotest/__snapshots__/{test_fixtures => test_snapshots}/test_binary_array_snapshot.npy (100%) rename autotest/__snapshots__/{test_fixtures => test_snapshots}/test_readable_text_array_snapshot.txt (100%) rename autotest/__snapshots__/{test_fixtures => test_snapshots}/test_text_array_snapshot.txt (100%) create mode 100644 autotest/test_snapshots.py create mode 100644 docs/md/snapshots.md create mode 100644 modflow_devtools/snapshots.py delete mode 100644 scripts/lint.py diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 890bdaf..1ff71fc 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -45,7 +45,6 @@ jobs: ref="${{ github.ref_name }}" version="${ref#"v"}" python scripts/update_version.py -v "$version" - python scripts/lint.py python -c "import modflow_devtools; print('Version: ', modflow_devtools.__version__)" echo "version=$version" >> $GITHUB_OUTPUT diff --git a/README.md b/README.md index b81da4a..726b0f6 100644 --- a/README.md +++ b/README.md @@ -74,7 +74,7 @@ pip install "modflow-devtools[test]" To install from source and set up a development environment please see the [developer documentation](DEVELOPER.md). -To import `pytest` fixtures in a project consuming `modflow-devtools`, add the following to a `conftest.py` file: +To import `pytest` fixtures in a project consuming `modflow-devtools`, add the following to a test file or `conftest.py` file: ```python pytest_plugins = [ "modflow_devtools.fixtures" ] diff --git a/autotest/__snapshots__/test_fixtures/test_binary_array_snapshot.npy b/autotest/__snapshots__/test_snapshots/test_binary_array_snapshot.npy similarity index 100% rename from autotest/__snapshots__/test_fixtures/test_binary_array_snapshot.npy rename to autotest/__snapshots__/test_snapshots/test_binary_array_snapshot.npy diff --git a/autotest/__snapshots__/test_fixtures/test_readable_text_array_snapshot.txt b/autotest/__snapshots__/test_snapshots/test_readable_text_array_snapshot.txt similarity index 100% rename from autotest/__snapshots__/test_fixtures/test_readable_text_array_snapshot.txt rename to autotest/__snapshots__/test_snapshots/test_readable_text_array_snapshot.txt diff --git a/autotest/__snapshots__/test_fixtures/test_text_array_snapshot.txt b/autotest/__snapshots__/test_snapshots/test_text_array_snapshot.txt similarity index 100% rename from autotest/__snapshots__/test_fixtures/test_text_array_snapshot.txt rename to autotest/__snapshots__/test_snapshots/test_text_array_snapshot.txt diff --git a/autotest/test_fixtures.py b/autotest/test_fixtures.py index 600a34c..b9fa5b5 100644 --- a/autotest/test_fixtures.py +++ b/autotest/test_fixtures.py @@ -2,7 +2,6 @@ import platform from pathlib import Path -import numpy as np import pytest from _pytest.config import ExitCode @@ -303,63 +302,3 @@ def test_tabular(tabular, arg, function_tmpdir): assert pytest.main(args) == ExitCode.OK res = open(next(function_tmpdir.rglob(test_tabular_fname))).readlines()[0] assert tabular == res - - -# test snapshot fixtures - - -snapshot_array = np.array([1.1, 2.2, 3.3]) -snapshots_path = proj_root / "autotest" / "__snapshots__" - - -def test_binary_array_snapshot(array_snapshot): - assert array_snapshot == snapshot_array - snapshot_path = ( - snapshots_path - / module_path.stem - / f"{inspect.currentframe().f_code.co_name}.npy" - ) - assert snapshot_path.is_file() - assert np.allclose(np.load(snapshot_path), snapshot_array) - - -# todo: reinstate if/when we support multiple arrays -# def test_binary_array_snapshot_multi(array_snapshot): -# arrays = {"ascending": snapshot_array, "descending": np.flip(snapshot_array)} -# assert array_snapshot == arrays -# snapshot_path = ( -# snapshots_path -# / module_path.stem -# / f"{inspect.currentframe().f_code.co_name}.npy" -# ) -# assert snapshot_path.is_file() -# assert np.allclose(np.load(snapshot_path)["ascending"], snapshot_array) -# assert np.allclose(np.load(snapshot_path)["descending"], np.flip(snapshot_array)) - - -def test_text_array_snapshot(text_array_snapshot): - assert text_array_snapshot == snapshot_array - snapshot_path = ( - snapshots_path - / module_path.stem - / f"{inspect.currentframe().f_code.co_name}.txt" - ) - assert snapshot_path.is_file() - assert np.allclose(np.loadtxt(snapshot_path), snapshot_array) - - -def test_readable_text_array_snapshot(readable_array_snapshot): - assert readable_array_snapshot == snapshot_array - snapshot_path = ( - snapshots_path - / module_path.stem - / f"{inspect.currentframe().f_code.co_name}.txt" - ) - assert snapshot_path.is_file() - assert np.allclose( - np.fromstring( - open(snapshot_path).readlines()[0].replace("[", "").replace("]", ""), - sep=" ", - ), - snapshot_array, - ) diff --git a/autotest/test_snapshots.py b/autotest/test_snapshots.py new file mode 100644 index 0000000..04cabc9 --- /dev/null +++ b/autotest/test_snapshots.py @@ -0,0 +1,63 @@ +import inspect +from pathlib import Path + +import numpy as np + +proj_root = Path(__file__).parents[1] +module_path = Path(inspect.getmodulename(__file__)) +pytest_plugins = [ "modflow_devtools.snapshots" ] # activate snapshot fixtures +snapshot_array = np.array([1.1, 2.2, 3.3]) +snapshots_path = proj_root / "autotest" / "__snapshots__" + + +def test_binary_array_snapshot(array_snapshot): + assert array_snapshot == snapshot_array + snapshot_path = ( + snapshots_path + / module_path.stem + / f"{inspect.currentframe().f_code.co_name}.npy" + ) + assert snapshot_path.is_file() + assert np.allclose(np.load(snapshot_path), snapshot_array) + + +# todo: reinstate if/when we support multiple arrays +# def test_binary_array_snapshot_multi(array_snapshot): +# arrays = {"ascending": snapshot_array, "descending": np.flip(snapshot_array)} +# assert array_snapshot == arrays +# snapshot_path = ( +# snapshots_path +# / module_path.stem +# / f"{inspect.currentframe().f_code.co_name}.npy" +# ) +# assert snapshot_path.is_file() +# assert np.allclose(np.load(snapshot_path)["ascending"], snapshot_array) +# assert np.allclose(np.load(snapshot_path)["descending"], np.flip(snapshot_array)) + + +def test_text_array_snapshot(text_array_snapshot): + assert text_array_snapshot == snapshot_array + snapshot_path = ( + snapshots_path + / module_path.stem + / f"{inspect.currentframe().f_code.co_name}.txt" + ) + assert snapshot_path.is_file() + assert np.allclose(np.loadtxt(snapshot_path), snapshot_array) + + +def test_readable_text_array_snapshot(readable_array_snapshot): + assert readable_array_snapshot == snapshot_array + snapshot_path = ( + snapshots_path + / module_path.stem + / f"{inspect.currentframe().f_code.co_name}.txt" + ) + assert snapshot_path.is_file() + assert np.allclose( + np.fromstring( + open(snapshot_path).readlines()[0].replace("[", "").replace("]", ""), + sep=" ", + ), + snapshot_array, + ) diff --git a/docs/md/fixtures.md b/docs/md/fixtures.md index 613fd96..b847f10 100644 --- a/docs/md/fixtures.md +++ b/docs/md/fixtures.md @@ -169,13 +169,3 @@ Model-loading fixtures use a set of utility functions to find and enumerate mode - `get_namefile_paths()` These functions are used internally in a `pytest_generate_tests` hook to implement the above model-parametrization fixtures. See `fixtures.py` and/or this project's test suite for usage examples. - -## Snapshot testing - -Snapshot testing is a form of regression testing in which a "snapshot" of the results of some computation is verified and captured by the developer to be compared against when tests are subsequently run. This is accomplished with [`syrupy`](https://github.com/tophat/syrupy), which provides a `snapshot` fixture overriding the equality operator to allow comparison with e.g. `snapshot == result`. A few custom fixtures for snapshots of NumPy arrays are also provided: - -- `array_snapshot`: saves an array in a binary file for compact storage, can be inspected programmatically with `np.load()` -- `text_array_snapshot`: flattens an array and stores it in a text file, compromise between readability and disk usage -- `readable_array_snapshot`: stores an array in a text file in its original shape, easy to inspect but largest on disk - -By default, tests run in comparison mode. This means a newly written test using any of the snapshot fixtures will fail until a snapshot is created. Snapshots can be created/updated by running pytest with the `--snapshot-update` flag. \ No newline at end of file diff --git a/docs/md/snapshots.md b/docs/md/snapshots.md new file mode 100644 index 0000000..0254617 --- /dev/null +++ b/docs/md/snapshots.md @@ -0,0 +1,17 @@ +# Snapshot testing + +Snapshot testing is a form of regression testing in which a "snapshot" of the results of some computation is verified and captured by the developer to be compared against when tests are subsequently run. This is accomplished with [`syrupy`](https://github.com/tophat/syrupy), which provides a `snapshot` fixture overriding the equality operator to allow comparison with e.g. `snapshot == result`. A few custom fixtures for snapshots of NumPy arrays are also provided: + +- `array_snapshot`: saves an array in a binary file for compact storage, can be inspected programmatically with `np.load()` +- `text_array_snapshot`: flattens an array and stores it in a text file, compromise between readability and disk usage +- `readable_array_snapshot`: stores an array in a text file in its original shape, easy to inspect but largest on disk + +By default, tests run in comparison mode. This means a newly written test using any of the snapshot fixtures will fail until a snapshot is created. Snapshots can be created/updated by running pytest with the `--snapshot-update` flag. + +## Using snapshot fixtures + +To use snapshot fixtures, add the following line to a test file or `conftest.py` file: + +```python +pytest_plugins = [ "modflow_devtools.snapshots" ] +``` \ No newline at end of file diff --git a/modflow_devtools/fixtures.py b/modflow_devtools/fixtures.py index 2d37328..963fa4e 100644 --- a/modflow_devtools/fixtures.py +++ b/modflow_devtools/fixtures.py @@ -1,5 +1,4 @@ from collections import OrderedDict -from io import BytesIO, StringIO from itertools import groupby from os import PathLike, environ from pathlib import Path @@ -9,91 +8,7 @@ from modflow_devtools.imports import import_optional_dependency from modflow_devtools.misc import get_namefile_paths, get_packages -np = import_optional_dependency("numpy") pytest = import_optional_dependency("pytest") -syrupy = import_optional_dependency("syrupy") - -# ruff: noqa: E402 -from syrupy.extensions.single_file import ( - SingleFileSnapshotExtension, - WriteMode, -) -from syrupy.types import ( - PropertyFilter, - PropertyMatcher, - SerializableData, - SerializedData, -) - -# snapshot extensions - - -class BinaryArrayExtension(SingleFileSnapshotExtension): - """ - Binary snapshot of a NumPy array. Can be read back into NumPy with - .load(), preserving dtype and shape. This is the recommended array - snapshot approach if human-readability is not a necessity, as disk - space is minimized. - """ - - _write_mode = WriteMode.BINARY - _file_extension = "npy" - - def serialize( - self, - data, - *, - exclude=None, - include=None, - matcher=None, - ): - buffer = BytesIO() - np.save(buffer, data) - return buffer.getvalue() - - -class TextArrayExtension(SingleFileSnapshotExtension): - """ - Text snapshot of a NumPy array. Flattens the array before writing. - Can be read back into NumPy with .loadtxt() assuming you know the - shape of the expected data and subsequently reshape it if needed. - """ - - _write_mode = WriteMode.TEXT - _file_extension = "txt" - - def serialize( - self, - data: "SerializableData", - *, - exclude: Optional["PropertyFilter"] = None, - include: Optional["PropertyFilter"] = None, - matcher: Optional["PropertyMatcher"] = None, - ) -> "SerializedData": - buffer = StringIO() - np.savetxt(buffer, data.ravel()) - return buffer.getvalue() - - -class ReadableArrayExtension(SingleFileSnapshotExtension): - """ - Human-readable snapshot of a NumPy array. Preserves array shape - at the expense of possible loss of precision (default 8 places) - and more difficulty loading into NumPy than TextArrayExtension. - """ - - _write_mode = WriteMode.TEXT - _file_extension = "txt" - - def serialize( - self, - data: "SerializableData", - *, - exclude: Optional["PropertyFilter"] = None, - include: Optional["PropertyFilter"] = None, - matcher: Optional["PropertyMatcher"] = None, - ) -> "SerializedData": - return np.array2string(data, threshold=np.inf) # fixtures @@ -176,21 +91,6 @@ def tabular(request) -> str: return tab -@pytest.fixture -def array_snapshot(snapshot): - return snapshot.use_extension(BinaryArrayExtension) - - -@pytest.fixture -def text_array_snapshot(snapshot): - return snapshot.use_extension(TextArrayExtension) - - -@pytest.fixture -def readable_array_snapshot(snapshot): - return snapshot.use_extension(ReadableArrayExtension) - - # configuration hooks diff --git a/modflow_devtools/snapshots.py b/modflow_devtools/snapshots.py new file mode 100644 index 0000000..eed8776 --- /dev/null +++ b/modflow_devtools/snapshots.py @@ -0,0 +1,108 @@ +from io import BytesIO, StringIO +from typing import Optional + +from modflow_devtools.imports import import_optional_dependency + +np = import_optional_dependency("numpy") +pytest = import_optional_dependency("pytest") +syrupy = import_optional_dependency("syrupy") + +# ruff: noqa: E402 +from syrupy.extensions.single_file import ( + SingleFileSnapshotExtension, + WriteMode, +) +from syrupy.types import ( + PropertyFilter, + PropertyMatcher, + SerializableData, + SerializedData, +) + +# extension classes + + +class BinaryArrayExtension(SingleFileSnapshotExtension): + """ + Binary snapshot of a NumPy array. Can be read back into NumPy with + .load(), preserving dtype and shape. This is the recommended array + snapshot approach if human-readability is not a necessity, as disk + space is minimized. + """ + + _write_mode = WriteMode.BINARY + _file_extension = "npy" + + def serialize( + self, + data, + *, + exclude=None, + include=None, + matcher=None, + ): + buffer = BytesIO() + np.save(buffer, data) + return buffer.getvalue() + + +class TextArrayExtension(SingleFileSnapshotExtension): + """ + Text snapshot of a NumPy array. Flattens the array before writing. + Can be read back into NumPy with .loadtxt() assuming you know the + shape of the expected data and subsequently reshape it if needed. + """ + + _write_mode = WriteMode.TEXT + _file_extension = "txt" + + def serialize( + self, + data: "SerializableData", + *, + exclude: Optional["PropertyFilter"] = None, + include: Optional["PropertyFilter"] = None, + matcher: Optional["PropertyMatcher"] = None, + ) -> "SerializedData": + buffer = StringIO() + np.savetxt(buffer, data.ravel()) + return buffer.getvalue() + + +class ReadableArrayExtension(SingleFileSnapshotExtension): + """ + Human-readable snapshot of a NumPy array. Preserves array shape + at the expense of possible loss of precision (default 8 places) + and more difficulty loading into NumPy than TextArrayExtension. + """ + + _write_mode = WriteMode.TEXT + _file_extension = "txt" + + def serialize( + self, + data: "SerializableData", + *, + exclude: Optional["PropertyFilter"] = None, + include: Optional["PropertyFilter"] = None, + matcher: Optional["PropertyMatcher"] = None, + ) -> "SerializedData": + return np.array2string(data, threshold=np.inf) + + +# fixtures + + +@pytest.fixture +def array_snapshot(snapshot): + return snapshot.use_extension(BinaryArrayExtension) + + +@pytest.fixture +def text_array_snapshot(snapshot): + return snapshot.use_extension(TextArrayExtension) + + +@pytest.fixture +def readable_array_snapshot(snapshot): + return snapshot.use_extension(ReadableArrayExtension) diff --git a/scripts/lint.py b/scripts/lint.py deleted file mode 100644 index 315500a..0000000 --- a/scripts/lint.py +++ /dev/null @@ -1,21 +0,0 @@ -import os - -try: - import isort - - print(f"isort version: {isort.__version__}") -except ModuleNotFoundError: - print("isort not installed\n\tInstall using pip install isort") - -try: - import black - - print(f"black version: {black.__version__}") -except ModuleNotFoundError: - print("black not installed\n\tInstall using pip install black") - -print("running isort...") -os.system("isort -v .") - -print("running black...") -os.system("black -v .") From 3d3bb597861f20c499c1ec49c39b432877670306 Mon Sep 17 00:00:00 2001 From: wpbonelli Date: Wed, 15 May 2024 12:28:59 -0400 Subject: [PATCH 13/14] docs: add snapshots section to index.rst (#153) --- docs/index.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/index.rst b/docs/index.rst index 7e97db0..910c197 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -21,6 +21,7 @@ The `modflow-devtools` package provides a set of tools for developing and testin md/fixtures.md md/markers.md + md/snapshots.md .. toctree:: From 9302fd8b4d91ffb15f1dd1f26b9de81707a11003 Mon Sep 17 00:00:00 2001 From: "github-actions[bot]" <41898282+github-actions[bot]@users.noreply.github.com> Date: Wed, 15 May 2024 16:44:57 +0000 Subject: [PATCH 14/14] ci(release): set version to 1.5.0, update changelog --- HISTORY.md | 12 ++++++++++++ docs/conf.py | 2 +- modflow_devtools/__init__.py | 4 ++-- version.txt | 2 +- version.txt.lock | 0 5 files changed, 16 insertions(+), 4 deletions(-) create mode 100644 version.txt.lock diff --git a/HISTORY.md b/HISTORY.md index 5ee3c8e..4f87fb5 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -1,3 +1,15 @@ +### Version 1.5.0 + +#### New features + +* [feat(markers)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/1f358de2bc721c1000c3d0823b9440776432e3b0): Add no_parallel marker, support differing pkg/module names (#148). Committed by wpbonelli on 2024-04-12. +* [feat(snapshots)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/c9e445dd1544413f3729c7a78c2a77038db80050): Add snapshot fixtures, remove pandas fixture (#151). Committed by wpbonelli on 2024-05-13. + +#### Refactoring + +* [refactor(latex)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/827b5ec63ebe0b9ea833957637d6b60fdc2f3198): Support path-like, add docstrings (#142). Committed by wpbonelli on 2024-02-25. +* [refactor(snapshots)](https://github.com/MODFLOW-USGS/modflow-devtools/commit/d96089e512fbb79408e4fb58c89ee63da60dc727): Move to separate module (#152). Committed by wpbonelli on 2024-05-13. + ### Version 1.4.0 #### New features diff --git a/docs/conf.py b/docs/conf.py index 9fd38eb..63390be 100644 --- a/docs/conf.py +++ b/docs/conf.py @@ -8,7 +8,7 @@ project = "modflow-devtools" author = "MODFLOW Team" -release = "1.5.0.dev0" +release = '1.5.0' # -- General configuration --------------------------------------------------- # https://www.sphinx-doc.org/en/master/usage/configuration.html#general-configuration diff --git a/modflow_devtools/__init__.py b/modflow_devtools/__init__.py index f8f2600..7614347 100644 --- a/modflow_devtools/__init__.py +++ b/modflow_devtools/__init__.py @@ -1,6 +1,6 @@ __author__ = "Joseph D. Hughes" -__date__ = "Feb 19, 2024" -__version__ = "1.5.0.dev0" +__date__ = "May 15, 2024" +__version__ = "1.5.0" __maintainer__ = "Joseph D. Hughes" __email__ = "jdhughes@usgs.gov" __status__ = "Production" diff --git a/version.txt b/version.txt index 47ed95f..3e1ad72 100644 --- a/version.txt +++ b/version.txt @@ -1 +1 @@ -1.5.0.dev0 \ No newline at end of file +1.5.0 \ No newline at end of file diff --git a/version.txt.lock b/version.txt.lock new file mode 100644 index 0000000..e69de29