refactor: use pathlib and subprocess; refactor other aspects (#166)

There are several refactors combined in this:

* Use pathlib more consistently (also enforced with Ruff's PTH rule (except for modflow_devtools/zip.py)
* Refactor some aspects of subprocess.run() in meson_build() to not use shell=True. Also, run meson compile ... before meson install ...
* Apply some automated fixes from Ruff rules C4, RUF and UP.
* Fix missing parameter in test_fixtures.py::test_keep_function_scoped_tmpdir (found via Ruff's ARG rule)

autotest/test_download.py

@@ -53,9 +53,9 @@ def test_get_release(repo):
     if repo == "MODFLOW-USGS/modflow6":
         # 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")]
-        )
+        assert {a.rpartition("_")[2] for a in actual_names} >= {
+            a for a in expected_names if not a.startswith("win")
+        }
     else:
         assert set(actual_names) >= set(expected_names)
 
@@ -64,7 +64,7 @@ def test_get_release(repo):
 @requires_github
 @pytest.mark.parametrize("name", [None, "rtd-files", "run-time-comparison"])
 @pytest.mark.parametrize("per_page", [None, 100])
-def test_list_artifacts(tmp_path, name, per_page):
+def test_list_artifacts(name, per_page):
     artifacts = list_artifacts(
         "MODFLOW-USGS/modflow6",
         name=name,

autotest/test_fixtures.py

@@ -63,14 +63,14 @@ class TestClassScopedTmpdir:
 
     @pytest.fixture(autouse=True)
     def setup(self, class_tmpdir):
-        with open(class_tmpdir / self.fname, "w") as file:
-            file.write("hello, class-scoped tmpdir")
+        file = class_tmpdir / self.fname
+        file.write_text("hello, class-scoped tmpdir")
 
     def test_class_scoped_tmpdir(self, class_tmpdir):
         assert isinstance(class_tmpdir, Path)
         assert class_tmpdir.is_dir()
         assert self.__class__.__name__ in class_tmpdir.stem
-        assert Path(class_tmpdir / self.fname).is_file()
+        assert (class_tmpdir / self.fname).is_file()
 
 
 def test_module_scoped_tmpdir(module_tmpdir):
@@ -91,33 +91,33 @@ def test_session_scoped_tmpdir(session_tmpdir):
 
 @pytest.mark.meta("test_keep")
 def test_keep_function_scoped_tmpdir_inner(function_tmpdir):
-    with open(function_tmpdir / test_keep_fname, "w") as f:
-        f.write("hello, function-scoped tmpdir")
+    file = function_tmpdir / test_keep_fname
+    file.write_text("hello, function-scoped tmpdir")
 
 
 @pytest.mark.meta("test_keep")
 class TestKeepClassScopedTmpdirInner:
     def test_keep_class_scoped_tmpdir_inner(self, class_tmpdir):
-        with open(class_tmpdir / test_keep_fname, "w") as f:
-            f.write("hello, class-scoped tmpdir")
+        file = class_tmpdir / test_keep_fname
+        file.write_text("hello, class-scoped tmpdir")
 
 
 @pytest.mark.meta("test_keep")
 def test_keep_module_scoped_tmpdir_inner(module_tmpdir):
-    with open(module_tmpdir / test_keep_fname, "w") as f:
-        f.write("hello, module-scoped tmpdir")
+    file = module_tmpdir / test_keep_fname
+    file.write_text("hello, module-scoped tmpdir")
 
 
 @pytest.mark.meta("test_keep")
 def test_keep_session_scoped_tmpdir_inner(session_tmpdir):
-    with open(session_tmpdir / test_keep_fname, "w") as f:
-        f.write("hello, session-scoped tmpdir")
+    file = session_tmpdir / test_keep_fname
+    file.write_text("hello, session-scoped tmpdir")
 
 
 @pytest.mark.parametrize("arg", ["--keep", "-K"])
 def test_keep_function_scoped_tmpdir(function_tmpdir, arg):
     inner_fn = test_keep_function_scoped_tmpdir_inner.__name__
-    file_path = Path(function_tmpdir / f"{inner_fn}0" / test_keep_fname)
+    file_path = function_tmpdir / f"{inner_fn}0" / test_keep_fname
     args = [
         __file__,
         "-v",
@@ -126,7 +126,7 @@ def test_keep_function_scoped_tmpdir(function_tmpdir, arg):
         inner_fn,
         "-M",
         "test_keep",
-        "-K",
+        arg,
         function_tmpdir,
     ]
     assert pytest.main(args) == ExitCode.OK
@@ -155,7 +155,7 @@ def test_keep_class_scoped_tmpdir(tmp_path, arg):
         tmp_path,
     ]
     assert pytest.main(args) == ExitCode.OK
-    assert Path(
+    assert (
         tmp_path / f"{TestKeepClassScopedTmpdirInner.__name__}0" / test_keep_fname
     ).is_file()
 
@@ -175,7 +175,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"{this_path.parent.name}.{this_path.stem}0"
     assert test_keep_fname in [f.name for f in keep_path.glob("*")]
 
 
@@ -193,17 +193,15 @@ def test_keep_session_scoped_tmpdir(tmp_path, arg, request):
         tmp_path,
     ]
     assert pytest.main(args) == ExitCode.OK
-    assert Path(
-        tmp_path / f"{request.config.rootpath.name}0" / test_keep_fname
-    ).is_file()
+    assert (tmp_path / f"{request.config.rootpath.name}0" / test_keep_fname).is_file()
 
 
 @pytest.mark.meta("test_keep_failed")
 def test_keep_failed_function_scoped_tmpdir_inner(function_tmpdir):
-    with open(function_tmpdir / test_keep_fname, "w") as f:
-        f.write("hello, function-scoped tmpdir")
+    file = function_tmpdir / test_keep_fname
+    file.write_text("hello, function-scoped tmpdir")
 
-    assert False, "oh no"
+    raise AssertionError("oh no")
 
 
 @pytest.mark.parametrize("keep", [True, False])
@@ -214,7 +212,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 = (function_tmpdir / f"{inner_fn}0" / test_keep_fname).is_file()
     assert kept_file if keep else not kept_file
 
 
@@ -290,8 +288,8 @@ def test_large_test_model(large_test_model):
 
 @pytest.mark.meta("test_tabular")
 def test_tabular_inner(function_tmpdir, tabular):
-    with open(function_tmpdir / test_tabular_fname, "w") as f:
-        f.write(str(tabular))
+    file = function_tmpdir / test_tabular_fname
+    file.write_text(str(tabular))
 
 
 @pytest.mark.parametrize("tabular", ["raw", "recarray", "dataframe"])
@@ -312,5 +310,5 @@ def test_tabular(tabular, arg, function_tmpdir):
         "test_tabular",
     ]
     assert pytest.main(args) == ExitCode.OK
-    res = open(next(function_tmpdir.rglob(test_tabular_fname))).readlines()[0]
-    assert tabular == res
+    file = next(function_tmpdir.rglob(test_tabular_fname))
+    assert tabular == file.read_text()

autotest/test_misc.py

@@ -22,10 +22,10 @@
 
 
 def test_set_dir(tmp_path):
-    assert Path(os.getcwd()) != tmp_path
+    assert Path.cwd() != tmp_path
     with set_dir(tmp_path):
-        assert Path(os.getcwd()) == tmp_path
-    assert Path(os.getcwd()) != tmp_path
+        assert Path.cwd() == tmp_path
+    assert Path.cwd() != tmp_path
 
 
 def test_set_env():
@@ -49,17 +49,13 @@ 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_mf5to6 = sorted(
-    list((_testmodels_repo_path / "mf5to6").glob("test*"))
-)
+_testmodels_repo_paths_mf6 = sorted((_testmodels_repo_path / "mf6").glob("test*"))
+_testmodels_repo_paths_mf5to6 = sorted((_testmodels_repo_path / "mf5to6").glob("test*"))
 _largetestmodels_repo_path = _repos_path / "modflow6-largetestmodels"
-_largetestmodel_paths = sorted(list(_largetestmodels_repo_path.glob("test*")))
+_largetestmodel_paths = sorted(_largetestmodels_repo_path.glob("test*"))
 _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 []
-)
+_example_paths = sorted(_examples_path.glob("ex-*")) if _examples_path.is_dir() else []
 
 
 @pytest.mark.skipif(
@@ -95,20 +91,19 @@ def test_get_packages_fails_on_invalid_namefile(module_tmpdir):
 
     # 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:
-            if "GWF6" in line:
-                line = line.replace("GWF6", "GWF6  garbage")
-            f.write(line + os.linesep)
-    assert set(get_packages(namefile_path)) == {"gwf", "tdis", "ims"}
+    lines = []
+    for line in namefile_path.read_text().splitlines():
+        if "GWF6" in line:
+            line = line.replace("GWF6", "GWF6  garbage")
+        lines.append(line)
+    namefile_path.write_text("\n".join(lines))
+
+    with pytest.warns(UserWarning, match="Invalid namefile format"):
+        assert set(get_packages(namefile_path)) == {"gwf", "tdis", "ims"}
 
     # entirely unparsable namefile - result should be empty
-    lines = open(namefile_path, "r").read().splitlines()
-    with open(namefile_path, "w") as f:
-        for _ in lines:
-            f.write("garbage" + os.linesep)
-    assert not any(get_packages(namefile_path))
+    namefile_path.write_text("garbage\n" * 20)
+    assert get_packages(namefile_path) == []
 
 
 @pytest.mark.skipif(not any(_example_paths), reason="examples not found")
@@ -129,34 +124,34 @@ def test_has_package():
 
 def get_expected_model_dirs(path, pattern="mfsim.nam") -> List[Path]:
     folders = []
-    for root, dirs, files in os.walk(path):
+    for root, dirs, _ in os.walk(path):
         for d in dirs:
             p = Path(root) / d
             if any(p.glob(pattern)):
                 folders.append(p)
-    return sorted(list(set(folders)))
+    return sorted(set(folders))
 
 
 def get_expected_namefiles(path, pattern="mfsim.nam") -> List[Path]:
     folders = []
-    for root, dirs, files in os.walk(path):
+    for root, dirs, _ in os.walk(path):
         for d in dirs:
             p = Path(root) / d
             found = list(p.glob(pattern))
             folders = folders + found
-    return sorted(list(set(folders)))
+    return sorted(set(folders))
 
 
 @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)
-    assert sorted(paths) == sorted(list(set(paths)))  # no duplicates
+    assert sorted(paths) == sorted(set(paths))  # no duplicates
     assert set(expected_paths) == set(paths)
 
     expected_paths = get_expected_model_dirs(_examples_path, "*.nam")
     paths = get_model_paths(_examples_path, namefile="*.nam")
-    assert sorted(paths) == sorted(list(set(paths)))
+    assert sorted(paths) == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
 
@@ -166,12 +161,12 @@ def test_get_model_paths_examples():
 def test_get_model_paths_largetestmodels():
     expected_paths = get_expected_model_dirs(_examples_path)
     paths = get_model_paths(_examples_path)
-    assert sorted(paths) == sorted(list(set(paths)))
+    assert sorted(paths) == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
     expected_paths = get_expected_model_dirs(_examples_path)
     paths = get_model_paths(_examples_path)
-    assert sorted(paths) == sorted(list(set(paths)))
+    assert sorted(paths) == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
 
@@ -192,12 +187,12 @@ def test_get_model_paths_exclude_patterns(models):
 def test_get_namefile_paths_examples():
     expected_paths = get_expected_namefiles(_examples_path)
     paths = get_namefile_paths(_examples_path)
-    assert paths == sorted(list(set(paths)))
+    assert paths == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
     expected_paths = get_expected_namefiles(_examples_path, "*.nam")
     paths = get_namefile_paths(_examples_path, namefile="*.nam")
-    assert paths == sorted(list(set(paths)))
+    assert paths == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
 
@@ -207,12 +202,12 @@ def test_get_namefile_paths_examples():
 def test_get_namefile_paths_largetestmodels():
     expected_paths = get_expected_namefiles(_largetestmodels_repo_path)
     paths = get_namefile_paths(_largetestmodels_repo_path)
-    assert paths == sorted(list(set(paths)))
+    assert paths == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
     expected_paths = get_expected_namefiles(_largetestmodels_repo_path)
     paths = get_namefile_paths(_largetestmodels_repo_path)
-    assert paths == sorted(list(set(paths)))
+    assert paths == sorted(set(paths))
     assert set(expected_paths) == set(paths)
 
 

autotest/test_snapshots.py

@@ -56,10 +56,7 @@ def test_readable_text_array_snapshot(readable_array_snapshot):
     )
     assert snapshot_path.is_file()
     assert np.allclose(
-        np.fromstring(
-            open(snapshot_path).readlines()[0].replace("[", "").replace("]", ""),
-            sep=" ",
-        ),
+        np.fromstring(snapshot_path.read_text().strip("[]\r\n"), sep=" "),
         snapshot_array,
     )
 

autotest/test_zip.py

@@ -23,8 +23,8 @@ def test_compressall(function_tmpdir):
     zip_file = function_tmpdir / "output.zip"
     input_dir = function_tmpdir / "input"
     input_dir.mkdir()
-    with open(input_dir / "data.txt", "w") as f:
-        f.write("hello world")
+    file = input_dir / "data.txt"
+    file.write_text("hello world")
 
     MFZipFile.compressall(str(zip_file), dir_pths=str(input_dir))
     pprint(list(function_tmpdir.iterdir()))
@@ -42,8 +42,7 @@ 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"  # noqa: E501
     path = module_tmpdir / "empty.zip"
-    with open(path, "wb") as zip:
-        zip.write(data)
+    path.write_bytes(data)
     yield path