Fix import_path. (#424)

Author: tobiasraabe

docs/source/changes.md

@@ -40,6 +40,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and
   decorators.
 - {pull}`421` removes the deprecation warning when `produces` is used as an magic
   function keyword to define products.
+- {pull}`424` fixes problems with {func}`~_pytask.path.import_path`.
 
 ## 0.3.2 - 2023-06-07
 

src/_pytask/path.py

@@ -1,6 +1,7 @@
 """Contains code to handle paths."""
 from __future__ import annotations
 
+import contextlib
 import functools
 import importlib.util
 import os
@@ -122,6 +123,8 @@ def import_path(path: Path, root: Path) -> ModuleType:
 
     """
     module_name = _module_name_from_path(path, root)
+    with contextlib.suppress(KeyError):
+        return sys.modules[module_name]
 
     spec = importlib.util.spec_from_file_location(module_name, str(path))
 
@@ -154,6 +157,11 @@ def _module_name_from_path(path: Path, root: Path) -> str:
         # Use the parts for the relative path to the root path.
         path_parts = relative_path.parts
 
+    # Module name for packages do not contain the __init__ file, unless the
+    # `__init__.py` file is at the root.
+    if len(path_parts) >= 2 and path_parts[-1] == "__init__":  # noqa: PLR2004
+        path_parts = path_parts[:-1]
+
     return ".".join(path_parts)
 
 

tests/conftest.py

@@ -1,16 +1,77 @@
 from __future__ import annotations
 
+import sys
+from contextlib import contextmanager
 from pathlib import Path
+from typing import Callable
 
 import pytest
 from click.testing import CliRunner
 
 
-@pytest.fixture()
-def runner():
-    return CliRunner()
-
-
 @pytest.fixture(autouse=True)
 def _add_objects_to_doctest_namespace(doctest_namespace):
     doctest_namespace["Path"] = Path
+
+
+class SysPathsSnapshot:
+    """A snapshot for sys.path."""
+
+    def __init__(self) -> None:
+        self.__saved = list(sys.path), list(sys.meta_path)
+
+    def restore(self) -> None:
+        sys.path[:], sys.meta_path[:] = self.__saved
+
+
+class SysModulesSnapshot:
+    """A snapshot for sys.modules."""
+
+    def __init__(self, preserve: Callable[[str], bool] | None = None) -> None:
+        self.__preserve = preserve
+        self.__saved = dict(sys.modules)
+
+    def restore(self) -> None:
+        if self.__preserve:
+            self.__saved.update(
+                (k, m) for k, m in sys.modules.items() if self.__preserve(k)
+            )
+        sys.modules.clear()
+        sys.modules.update(self.__saved)
+
+
+@contextmanager
+def restore_sys_path_and_module_after_test_execution():
+    sys_path_snapshot = SysPathsSnapshot()
+    sys_modules_snapshot = SysModulesSnapshot()
+    yield
+    sys_modules_snapshot.restore()
+    sys_path_snapshot.restore()
+
+
+@pytest.fixture(autouse=True)
+def _restore_sys_path_and_module_after_test_execution():
+    """Restore sys.path and sys.modules after every test execution.
+
+    This fixture became necessary because most task modules in the tests are named
+    `task_example`. Since the change in #424, the same module is not reimported which
+    solves errors with parallelization. At the same time, modules with the same name in
+    the tests are overshadowing another and letting tests fail.
+
+    The changes to `sys.path` might not be necessary to restore, but we do it anyways.
+
+    """
+    with restore_sys_path_and_module_after_test_execution():
+        yield
+
+
+class CustomCliRunner(CliRunner):
+    def invoke(self, *args, **kwargs):
+        """Restore sys.path and sys.modules after an invocation."""
+        with restore_sys_path_and_module_after_test_execution():
+            return super().invoke(*args, **kwargs)
+
+
+@pytest.fixture()
+def runner():
+    return CustomCliRunner()

tests/test_path.py

@@ -124,33 +124,34 @@ def test_find_case_sensitive_path(tmp_path, path, existing_paths, expected):
 
 
 @pytest.fixture()
-def simple_module(tmp_path: Path) -> Path:
-    fn = tmp_path / "_src/project/mymod.py"
+def simple_module(request, tmp_path: Path) -> Path:
+    name = f"mymod_{request.node.name}"
+    fn = tmp_path / f"_src/project/{name}.py"
     fn.parent.mkdir(parents=True)
     fn.write_text("def foo(x): return 40 + x")
-    return fn
+    module_name = _module_name_from_path(fn, root=tmp_path)
+    yield fn
+    sys.modules.pop(module_name, None)
 
 
 @pytest.mark.unit()
-def test_importmode_importlib(simple_module: Path, tmp_path: Path) -> None:
+def test_importmode_importlib(request, simple_module: Path, tmp_path: Path) -> None:
     """`importlib` mode does not change sys.path."""
     module = import_path(simple_module, root=tmp_path)
     assert module.foo(2) == 42  # type: ignore[attr-defined]
     assert str(simple_module.parent) not in sys.path
     assert module.__name__ in sys.modules
-    assert module.__name__ == "_src.project.mymod"
+    assert module.__name__ == f"_src.project.mymod_{request.node.name}"
     assert "_src" in sys.modules
     assert "_src.project" in sys.modules
 
 
 @pytest.mark.unit()
-def test_importmode_twice_is_different_module(
-    simple_module: Path, tmp_path: Path
-) -> None:
-    """`importlib` mode always returns a new module."""
+def test_remembers_previous_imports(simple_module: Path, tmp_path: Path) -> None:
+    """importlib mode called remembers previous module (pytest#10341, pytest#10811)."""
     module1 = import_path(simple_module, root=tmp_path)
     module2 = import_path(simple_module, root=tmp_path)
-    assert module1 is not module2
+    assert module1 is module2
 
 
 @pytest.mark.unit()
@@ -165,6 +166,9 @@ def test_no_meta_path_found(
     # mode='importlib' fails if no spec is found to load the module
     import importlib.util
 
+    # Force module to be re-imported.
+    del sys.modules[module.__name__]
+
     monkeypatch.setattr(
         importlib.util, "spec_from_file_location", lambda *args: None  # noqa: ARG005
     )
@@ -288,6 +292,10 @@ def test_module_name_from_path(tmp_path: Path) -> None:
     result = _module_name_from_path(Path("/home/foo/task_foo.py"), Path("/bar"))
     assert result == "home.foo.task_foo"
 
+    # Importing __init__.py files should return the package as module name.
+    result = _module_name_from_path(tmp_path / "src/app/__init__.py", tmp_path)
+    assert result == "src.app"
+
 
 @pytest.mark.unit()
 def test_insert_missing_modules(
@@ -308,3 +316,42 @@ def test_insert_missing_modules(
     modules = {}
     _insert_missing_modules(modules, "")
     assert not modules
+
+
+def test_importlib_package(monkeypatch: pytest.MonkeyPatch, tmp_path: Path):
+    """
+    Importing a package using --importmode=importlib should not import the
+    package's __init__.py file more than once (#11306).
+    """
+    monkeypatch.chdir(tmp_path)
+    monkeypatch.syspath_prepend(tmp_path)
+
+    package_name = "importlib_import_package"
+    tmp_path.joinpath(package_name).mkdir()
+    init = tmp_path.joinpath(f"{package_name}/__init__.py")
+    init.write_text(
+        textwrap.dedent(
+            """
+            from .singleton import Singleton
+            instance = Singleton()
+            """
+        ),
+        encoding="ascii",
+    )
+    singleton = tmp_path.joinpath(f"{package_name}/singleton.py")
+    singleton.write_text(
+        textwrap.dedent(
+            """
+            class Singleton:
+                INSTANCES = []
+                def __init__(self) -> None:
+                    self.INSTANCES.append(self)
+                    if len(self.INSTANCES) > 1:
+                        raise RuntimeError("Already initialized")
+            """
+        ),
+        encoding="ascii",
+    )
+
+    mod = import_path(init, root=tmp_path)
+    assert len(mod.instance.INSTANCES) == 1

tests/test_persist.py

@@ -13,6 +13,8 @@
 from pytask import SkippedUnchanged
 from pytask import TaskOutcome
 
+from tests.conftest import restore_sys_path_and_module_after_test_execution
+
 
 class DummyClass:
     pass
@@ -46,7 +48,8 @@ def task_dummy(depends_on, produces):
     tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source))
     tmp_path.joinpath("in.txt").write_text("I'm not the reason you care.")
 
-    session = build(paths=tmp_path)
+    with restore_sys_path_and_module_after_test_execution():
+        session = build(paths=tmp_path)
 
     assert session.exit_code == ExitCode.OK
     assert len(session.execution_reports) == 1