clean-up and fixes

Author: vpratz

noxfile.py

@@ -2,6 +2,14 @@
 import argparse
 from pathlib import Path
 import os
+import tempfile
+import shutil
+
+
+def git_rev_parse(session, commit):
+    print(f"Converting provided commit '{commit}' to Git revision...")
+    rev = session.run("git", "rev-parse", commit, external=True, silent=True).strip()
+    return rev
 
 
 @nox.session
@@ -13,32 +21,68 @@ def save_and_load(session: nox.Session):
     In load mode, the stored models and outputs are loaded from disk, and old and new outputs are compared.
     This helps to detect breaking serialization between versions.
 
-    Important: The test code from the current checkout is used, not from the installed version.
+    Important: The test code from the current checkout, not from `commit`, is used.
     """
     # parse the arguments
     parser = argparse.ArgumentParser()
     # add subparsers for the two different commands
     subparsers = parser.add_subparsers(help="subcommand help", dest="mode")
     # save command
     parser_save = subparsers.add_parser("save")
-    parser_save.add_argument("commit", type=str, default=".")
+    parser_save.add_argument("--install", type=str, default=".", required=True, dest="commit")
     # load command, additional "from" argument
     parser_load = subparsers.add_parser("load")
-    parser_load.add_argument("commit", type=str, default=".")
-    parser.add_argument("--from", type=str, default="", required=False, dest="from_commit")
+    parser_load.add_argument("--from", type=str, required=True, dest="from_commit")
+    parser_load.add_argument("--install", type=str, required=True, dest="commit")
+
     # keep unknown arguments, they will be forwarded to pytest below
     args, unknownargs = parser.parse_known_args(session.posargs)
 
+    if args.mode == "load":
+        if args.from_commit == ".":
+            from_commit = "local"
+        else:
+            from_commit = git_rev_parse(session, args.from_commit)
+
+        from_path = Path("_compatibility_data").absolute() / from_commit
+        if not from_path.exists():
+            raise FileNotFoundError(
+                f"The directory {from_path} does not exist, cannot load data.\n"
+                f"Please run 'nox -- save {args.from_commit}' to create it, and then rerun this command."
+            )
+
+        print(f"Data will be loaded from path {from_path}.")
+
     # install dependencies, currently the jax backend is used, but we could add a configuration option for this
-    repo_path = Path(os.curdir).absolute().parent / "bf2"
-    session.install(f"git+file://{str(repo_path)}@{args.commit}")
+    repo_path = Path(os.curdir).absolute()
+    if args.commit == ".":
+        print("'.' provided, installing local state...")
+        if args.mode == "save":
+            print("Output will be saved to the alias 'local'")
+        commit = "local"
+        session.install(".[test]")
+    else:
+        commit = git_rev_parse(session, args.commit)
+        print("Installing specified revision...")
+        session.install(f"bayesflow[test] @ git+file://{str(repo_path)}@{commit}")
     session.install("jax")
-    session.install("pytest")
 
-    # pass mode and commits to pytest, required for correct save and load behavior
-    cmd = ["pytest", "--mode", args.mode, "--commit", args.commit]
-    if args.mode == "load":
-        cmd += ["--from", args.from_commit]
-    cmd += unknownargs
+    with tempfile.TemporaryDirectory() as tmpdirname:
+        # launch in temporary directory, as the local bayesflow would overshadow the installed one
+        tmpdirname = Path(tmpdirname)
+        # pass mode and data path to pytest, required for correct save and load behavior
+        if args.mode == "load":
+            data_path = from_path
+        else:
+            data_path = Path("_compatibility_data").absolute() / commit
+            if data_path.exists():
+                print(f"Removing existing data directory {data_path}...")
+                shutil.rmtree(data_path)
+
+        cmd = ["pytest", "tests/test_compatibility", f"--mode={args.mode}", f"--data-path={data_path}"]
+        cmd += unknownargs
 
-    session.run(*cmd, env={"KERAS_BACKEND": "jax"})
+        print(f"Copying tests from working directory to temporary directory: {tmpdirname}")
+        shutil.copytree("tests", tmpdirname / "tests")
+        with session.chdir(tmpdirname):
+            session.run(*cmd, env={"KERAS_BACKEND": "jax"})

pyproject.toml

@@ -55,6 +55,7 @@ all = [
     "sphinxcontrib-bibtex ~= 2.6",
     "snowballstemmer ~= 2.2.0",
     # test
+    "nox",
     "pytest",
     "pytest-cov",
     "pytest-rerunfailures",
@@ -81,6 +82,7 @@ test = [
     "nbconvert",
     "ipython",
     "ipykernel",
+    "nox",
     "pytest",
     "pytest-cov",
     "pytest-rerunfailures",

tests/conftest.py

@@ -8,8 +8,7 @@
 
 def pytest_addoption(parser):
     parser.addoption("--mode", choices=["save", "load"])
-    parser.addoption("--commit", type=str)
-    parser.addoption("--from", type=str, required=False, dest="from_")
+    parser.addoption("--data-path", type=str)
 
 
 def pytest_runtest_setup(item):
@@ -73,16 +72,16 @@ def feature_size(request):
 
 
 @pytest.fixture()
-def random_conditions(batch_size, conditions_size):
+def random_conditions(random_seed, batch_size, conditions_size):
     if conditions_size is None:
         return None
 
-    return keras.random.normal((batch_size, conditions_size))
+    return keras.random.normal((batch_size, conditions_size), seed=10)
 
 
 @pytest.fixture()
-def random_samples(batch_size, feature_size):
-    return keras.random.normal((batch_size, feature_size))
+def random_samples(random_seed, batch_size, feature_size):
+    return keras.random.normal((batch_size, feature_size), seed=20)
 
 
 @pytest.fixture(scope="function", autouse=True)
@@ -93,8 +92,8 @@ def random_seed():
 
 
 @pytest.fixture()
-def random_set(batch_size, set_size, feature_size):
-    return keras.random.normal((batch_size, set_size, feature_size))
+def random_set(random_seed, batch_size, set_size, feature_size):
+    return keras.random.normal((batch_size, set_size, feature_size), seed=30)
 
 
 @pytest.fixture(params=[2, 3])

tests/test_compatibility/conftest.py

@@ -10,30 +10,21 @@ def mode(request):
     return mode
 
 
-@pytest.fixture(scope="session")
-def commit(request):
-    return request.config.getoption("--commit")
-
-
-@pytest.fixture(scope="session")
-def from_commit(request):
-    return request.config.getoption("--from")
-
-
 @pytest.fixture(autouse=True, scope="session")
-def data_dir(request, commit, from_commit, tmp_path_factory):
+def data_dir(request, tmp_path_factory):
     # read config option to detect "unset" scenario
     mode = request.config.getoption("--mode")
-    if mode == "save":
-        path = Path(".").absolute() / "_compatibility_data" / commit
-        return path
+    path = request.config.getoption("--data-path")
+    if not mode:
+        # if mode is unset, save and load from a temporary directory
+        return Path(tmp_path_factory.mktemp("_compatibility_data"))
+    elif not path:
+        pytest.exit(reason="Please provide the --data-path argument for model saving/loading.")
     elif mode == "load":
-        path = Path(".").absolute() / "_compatibility_data" / from_commit
+        path = Path(path)
         if not path.exists():
             pytest.exit(reason=f"Load path '{path}' does not exist. Please specify a valid load path", returncode=1)
-        return path
-    # if mode is unset, save and load from a temporary directory
-    return Path(tmp_path_factory.mktemp("_compatibility_data"))
+    return path
 
 
 # reduce number of test configurations

tests/test_compatibility/test_networks/test_summary_networks/test_summary_networks.py

@@ -15,7 +15,7 @@
     ],
     indirect=True,
 )
-class TestInferenceNetwork(SaveLoadTest):
+class TestSummaryNetwork(SaveLoadTest):
     filenames = {
         "model": "model.keras",
         "output": "output.pickle",

tests/test_compatibility/utils/helpers.py

@@ -1,16 +1,32 @@
 import pytest
-from utils import get_valid_filename, get_path
+import hashlib
+import inspect
+from pathlib import Path
 
 
 class SaveLoadTest:
     filenames = {}
 
     @pytest.fixture(autouse=True)
     def filepaths(self, data_dir, mode, request):
-        prefix = get_valid_filename(request._pyfuncitem.name)
+        # this name contains the config for the test and is therefore a unique identifier
+        test_config_str = request._pyfuncitem.name
+        # hash it, as it could be too long for the file system
+        prefix = hashlib.sha1(test_config_str.encode("utf-8")).hexdigest()
+        # use path to test file as base, remove ".py" suffix
+        base_path = Path(inspect.getsourcefile(type(self))[:-3])
+        # add class name
+        directory = base_path / type(self).__name__
+        # only keep the path relative to the tests directory
+        directory = directory.relative_to(Path("tests").absolute())
+        directory = Path(data_dir) / directory
+
+        if mode == "save":
+            directory.mkdir(parents=True, exist_ok=True)
+
         files = {}
         for label, filename in self.filenames.items():
-            path = get_path(data_dir, f"{prefix}__{filename}", create=mode == "save")
+            path = directory / f"{prefix}__{filename}"
             if mode == "load" and not path.exists():
                 pytest.skip(f"Required file not available: {path}")
             files[label] = path

tests/test_compatibility/utils/io.py

@@ -1,32 +1,6 @@
-import inspect
 from keras.saving import deserialize_keras_object, serialize_keras_object
-from pathlib import Path
 import pickle
-import re
-
-
-def get_path(data_dir: Path | str = "", filename: str = "", *, create: bool = False) -> Path:
-    frame = inspect.stack()[1]
-    base_path = Path(inspect.stack()[1].filename[:-3])
-    function_name = frame.function
-    if "self" in frame[0].f_locals:
-        filepath = base_path / frame[0].f_locals["self"].__class__.__name__ / function_name
-    else:
-        filepath = base_path / function_name
-    filepath = Path(data_dir) / filepath.relative_to(Path("tests").absolute())
-    if create is True:
-        filepath.mkdir(parents=True, exist_ok=True)
-    if filename:
-        return filepath / filename
-    return filepath
-
-
-def get_valid_filename(name):
-    s = str(name).strip().replace(" ", "_")
-    s = re.sub(r"(?u)[^-\w.]", "_", s)
-    if s in {"", ".", ".."}:
-        raise ValueError("Could not derive file name from '%s'" % name)
-    return s
+from pathlib import Path
 
 
 def dump_path(object, filepath: Path | str):