Skip to content

Commit

Permalink
chore: switch to ruff, misc linting fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
wpbonelli committed Mar 11, 2024
1 parent 5c0ee37 commit 99384ba
Show file tree
Hide file tree
Showing 15 changed files with 115 additions and 197 deletions.
15 changes: 2 additions & 13 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -30,21 +30,10 @@ jobs:
cache-dependency-path: pyproject.toml

- name: Install Python packages
run: |
pip install .
pip install ".[lint]"
run: pip install ".[lint]"

- name: Run isort
run: isort --verbose --check --diff modflow_devtools

- name: Run black
run: black --check --diff modflow_devtools

- 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
run: ruff check modflow_devtools

build:
name: Build
Expand Down
4 changes: 1 addition & 3 deletions autotest/test_build.py
Original file line number Diff line number Diff line change
Expand Up @@ -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")
)


Expand Down
8 changes: 2 additions & 6 deletions autotest/test_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
15 changes: 4 additions & 11 deletions autotest/test_fixtures.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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()


Expand All @@ -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("*")]


Expand Down Expand Up @@ -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


Expand Down
26 changes: 22 additions & 4 deletions autotest/test_markers.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,17 +3,32 @@
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)
def test_require_exe():
assert which(exe)
require_exe(exe)
require_program(exe)
requires_program(exe)


exes = [exe, "python"]
Expand All @@ -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
Expand All @@ -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)
Expand All @@ -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)
24 changes: 8 additions & 16 deletions autotest/test_misc.py
Original file line number Diff line number Diff line change
Expand Up @@ -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*"))
)
Expand All @@ -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 []
)


Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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)):
Expand All @@ -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)
58 changes: 22 additions & 36 deletions modflow_devtools/download.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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 = (
Expand Down Expand Up @@ -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"]
Expand Down Expand Up @@ -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 (
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand All @@ -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}")
Expand Down Expand Up @@ -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:
Expand All @@ -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()
Expand Down
Loading

0 comments on commit 99384ba

Please sign in to comment.