diff --git a/docs/source/changes.md b/docs/source/changes.md index bcfd3d7d..ecbe2459 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -8,6 +8,8 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and ## 0.4.3 - 2023-11-xx - {pull}`483` simplifies the teardown of a task. +- {pull}`484` raises more informative error when directories instead of files are used + with path nodes. - {pull}`485` adds missing steps to unconfigure pytask after the job is done which caused flaky tests. diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index 419bf83e..be9d4d3d 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -303,6 +303,10 @@ def pytask_collect_task( """ +_TEMPLATE_ERROR_DIRECTORY: str = """\ +The path '{path}' points to a directory, although only files are allowed.""" + + @hookimpl(trylast=True) def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PNode: """Collect a node of a task as a :class:`pytask.PNode`. @@ -348,6 +352,9 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PN node.path, session.config["paths"] or (session.config["root"],) ) + if isinstance(node, PPathNode) and node.path.is_dir(): + raise ValueError(_TEMPLATE_ERROR_DIRECTORY.format(path=node.path)) + if isinstance(node, PNode): return node @@ -362,6 +369,10 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PN node, session.config["check_casing_of_paths"] ) name = shorten_path(node, session.config["paths"] or (session.config["root"],)) + + if isinstance(node, Path) and node.is_dir(): + raise ValueError(_TEMPLATE_ERROR_DIRECTORY.format(path=path)) + return PathNode(name=name, path=node) node_name = create_name_of_python_node(node_info) diff --git a/src/_pytask/execute.py b/src/_pytask/execute.py index 6da40172..c6641710 100644 --- a/src/_pytask/execute.py +++ b/src/_pytask/execute.py @@ -127,7 +127,7 @@ def pytask_execute_task_setup(session: Session, task: PTask) -> None: for dependency in session.dag.predecessors(task.signature): node = session.dag.nodes[dependency]["node"] if not node.state(): - msg = f"{task.name} requires missing node {node.name}." + msg = f"{task.name!r} requires missing node {node.name!r}." if IS_FILE_SYSTEM_CASE_SENSITIVE: msg += ( "\n\n(Hint: Your file-system is case-sensitive. Check the paths' " diff --git a/tests/test_collect.py b/tests/test_collect.py index f811c867..c10b493d 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -572,3 +572,47 @@ def task_example(path: Annotated[Path, Path("file.txt"), Product]) -> None: ... result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.COLLECTION_FAILED assert "is defined twice" in result.output + + +@pytest.mark.parametrize( + "node", + [ + "Path(__file__).parent", + "PathNode(name='path', path=Path(__file__).parent)", + "PickleNode(name='', path=Path(__file__).parent)", + ], +) +def test_error_when_path_dependency_is_directory(runner, tmp_path, node): + source = f""" + from pathlib import Path + from pytask import PickleNode, PathNode + + def task_example(path = {node}): ... + """ + tmp_path.joinpath("task_example.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.COLLECTION_FAILED + assert all(i in result.output for i in ("only", "files", "are", "allowed")) + + +@pytest.mark.parametrize( + "node", + [ + "Path(__file__).parent", + "PathNode(name='path', path=Path(__file__).parent)", + "PickleNode(name='', path=Path(__file__).parent)", + ], +) +def test_error_when_path_product_is_directory(runner, tmp_path, node): + source = f""" + from pathlib import Path + from pytask import PickleNode, Product, PathNode + from typing_extensions import Annotated + from typing import Any + + def task_example(path: Annotated[Any, Product] = {node}): ... + """ + tmp_path.joinpath("task_example.py").write_text(textwrap.dedent(source)) + result = runner.invoke(cli, [tmp_path.as_posix()]) + assert result.exit_code == ExitCode.COLLECTION_FAILED + assert all(i in result.output for i in ("only", "files", "are", "allowed")) diff --git a/tests/test_execute.py b/tests/test_execute.py index a23a78a0..8dcf6dab 100644 --- a/tests/test_execute.py +++ b/tests/test_execute.py @@ -1004,7 +1004,7 @@ def task_d(produces): result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.FAILED - assert "NodeNotFoundError: task_d.py::task_d requires" in result.output + assert "NodeNotFoundError: 'task_d.py::task_d' requires" in result.output @pytest.mark.end_to_end() @@ -1024,7 +1024,7 @@ def task_e(in1_: Annotated[Path, node1], in2_: Annotated[Any, node2]): ... result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.FAILED - assert "NodeNotFoundError: task_e.py::task_e requires" in result.output + assert "NodeNotFoundError: 'task_e.py::task_e' requires" in result.output assert "input1" in result.output @@ -1045,7 +1045,7 @@ def task_d(produces): result = runner.invoke(cli, [tmp_path.joinpath("src").as_posix()]) assert result.exit_code == ExitCode.FAILED - assert "NodeNotFoundError: task_d.py::task_d requires" in result.output + assert "NodeNotFoundError: 'task_d.py::task_d' requires" in result.output assert "bld/in.txt" in result.output