Fix errors and simplify collection.

Author: tobiasraabe

src/_pytask/collect_utils.py

@@ -1,7 +1,6 @@
 """This module provides utility functions for :mod:`_pytask.collect`."""
 from __future__ import annotations
 
-import inspect
 import itertools
 import uuid
 from pathlib import Path
@@ -79,9 +78,15 @@ def parse_nodes(
     session: Session, path: Path, name: str, obj: Any, parser: Callable[..., Any]
 ) -> Any:
     """Parse nodes from object."""
+    arg_name = parser.__name__
     objects = _extract_nodes_from_function_markers(obj, parser)
-    nodes = _convert_objects_to_node_dictionary(objects, parser.__name__)
-    nodes = tree_map(lambda x: _collect_old_dependencies(session, path, name, x), nodes)
+    nodes = _convert_objects_to_node_dictionary(objects, arg_name)
+    nodes = tree_map(
+        lambda x: _collect_decorator_nodes(
+            session, path, name, NodeInfo(arg_name, (), x)
+        ),
+        nodes,
+    )
     return nodes
 
 
@@ -211,10 +216,23 @@ def _merge_dictionaries(list_of_dicts: list[dict[Any, Any]]) -> dict[Any, Any]:
     return out
 
 
-def parse_dependencies_from_task_function(
+_ERROR_MULTIPLE_DEPENDENCY_DEFINITIONS = """"Dependencies are defined via \
+'@pytask.mark.depends_on' and as default arguments for the argument 'depends_on'. Use \
+only one way and not both.
+
+Hint: You do not need to use 'depends_on' since pytask v0.4. Every function argument \
+that is not a product is treated as a dependency. Read more about dependencies in the \
+documentation: https://tinyurl.com/yrezszr4.
+"""
+
+
+def parse_dependencies_from_task_function(  # noqa: C901
     session: Session, path: Path, name: str, obj: Any
 ) -> dict[str, Any]:
     """Parse dependencies from task function."""
+    has_depends_on_decorator = False
+    has_depends_on_argument = False
+
     if has_mark(obj, "depends_on"):
         nodes = parse_nodes(session, path, name, obj, depends_on)
         return {"depends_on": nodes}
@@ -224,14 +242,30 @@ def parse_dependencies_from_task_function(
     kwargs = {**signature_defaults, **task_kwargs}
     kwargs.pop("produces", None)
 
+    dependencies = {}
+    # Parse products from task decorated with @task and that uses produces.
+    if "depends_on" in kwargs:
+        has_depends_on_argument = True
+        dependencies["depends_on"] = tree_map(
+            lambda x: _collect_decorator_nodes(
+                session, path, name, NodeInfo(arg_name="depends_on", path=(), value=x)
+            ),
+            kwargs["depends_on"],
+        )
+
+    if has_depends_on_decorator and has_depends_on_argument:
+        raise NodeNotCollectedError(_ERROR_MULTIPLE_PRODUCT_DEFINITIONS)
+
     parameters_with_product_annot = _find_args_with_product_annotation(obj)
     parameters_with_node_annot = _find_args_with_node_annotation(obj)
 
-    dependencies = {}
     for parameter_name, value in kwargs.items():
         if parameter_name in parameters_with_product_annot:
             continue
 
+        if parameter_name == "depends_on":
+            continue
+
         if parameter_name in parameters_with_node_annot:
 
             def _evolve(x: Any) -> Any:
@@ -316,8 +350,7 @@ def parse_products_from_task_function(
 
     """
     has_produces_decorator = False
-    has_task_decorator = False
-    has_signature_default = False
+    has_produces_argument = False
     has_annotation = False
     out = {}
 
@@ -333,6 +366,7 @@ def parse_products_from_task_function(
 
     # Parse products from task decorated with @task and that uses produces.
     if "produces" in kwargs:
+        has_produces_argument = True
         collected_products = tree_map_with_path(
             lambda p, x: _collect_product(
                 session,
@@ -345,52 +379,26 @@ def parse_products_from_task_function(
         )
         out = {"produces": collected_products}
 
-    parameters = inspect.signature(obj).parameters
-
-    # Parse products from default arguments
-    if not has_mark(obj, "task") and "produces" in parameters:
-        parameter = parameters["produces"]
-        if parameter.default is not parameter.empty:
-            has_signature_default = True
-            # Use _collect_new_node to not collect strings.
-            collected_products = tree_map_with_path(
-                lambda p, x: _collect_product(
-                    session,
-                    path,
-                    name,
-                    NodeInfo(arg_name="produces", path=p, value=x),
-                    is_string_allowed=False,
-                ),
-                parameter.default,
-            )
-            out = {"produces": collected_products}
-
     parameters_with_product_annot = _find_args_with_product_annotation(obj)
     if parameters_with_product_annot:
         has_annotation = True
         for parameter_name in parameters_with_product_annot:
-            # Use _collect_new_node to not collect strings.
-            collected_products = tree_map_with_path(
-                lambda p, x: _collect_product(
-                    session,
-                    path,
-                    name,
-                    NodeInfo(parameter_name, p, x),  # noqa: B023
-                    is_string_allowed=False,
-                ),
-                kwargs[parameter_name],
-            )
-            out = {parameter_name: collected_products}
+            if parameter_name in kwargs:
+                # Use _collect_new_node to not collect strings.
+                collected_products = tree_map_with_path(
+                    lambda p, x: _collect_product(
+                        session,
+                        path,
+                        name,
+                        NodeInfo(parameter_name, p, x),  # noqa: B023
+                        is_string_allowed=False,
+                    ),
+                    kwargs[parameter_name],
+                )
+                out = {parameter_name: collected_products}
 
     if (
-        sum(
-            (
-                has_produces_decorator,
-                has_task_decorator,
-                has_signature_default,
-                has_annotation,
-            )
-        )
+        sum((has_produces_decorator, has_produces_argument, has_annotation))
         >= 2  # noqa: PLR2004
     ):
         raise NodeNotCollectedError(_ERROR_MULTIPLE_PRODUCT_DEFINITIONS)
@@ -416,8 +424,15 @@ def _find_args_with_product_annotation(func: Callable[..., Any]) -> list[str]:
     return args_with_product_annot
 
 
-def _collect_old_dependencies(
-    session: Session, path: Path, name: str, node: str | Path
+_ERROR_WRONG_TYPE_DECORATOR = """'@pytask.mark.depends_on', '@pytask.mark.produces', \
+and their function arguments can only accept values of type 'str' and 'pathlib.Path' \
+or the same values nested in tuples, lists, and dictionaries. Here, {node} has type \
+{node_type}.
+"""
+
+
+def _collect_decorator_nodes(
+    session: Session, path: Path, name: str, node_info: NodeInfo
 ) -> dict[str, MetaNode]:
     """Collect nodes for a task.
 
@@ -427,22 +442,26 @@ def _collect_old_dependencies(
         If the node could not collected.
 
     """
+    node = node_info.value
+
     if not isinstance(node, (str, Path)):
-        raise ValueError(
-            "'@pytask.mark.depends_on' and '@pytask.mark.produces' can only accept "
-            "values of type 'str' and 'pathlib.Path' or the same values nested in "
-            f"tuples, lists, and dictionaries. Here, {node} has type {type(node)}."
+        raise NodeNotCollectedError(
+            _ERROR_WRONG_TYPE_DECORATOR.format(node=node, node_type=type(node))
         )
 
     if isinstance(node, str):
         node = Path(node)
+        node_info = node_info._replace(value=node)
 
     collected_node = session.hook.pytask_collect_node(
-        session=session, path=path, node_info=NodeInfo("produces", (), node)
+        session=session, path=path, node_info=node_info
     )
     if collected_node is None:
+        kind = {"depends_on": "dependency", "produces": "product"}.get(
+            node_info.arg_name
+        )
         raise NodeNotCollectedError(
-            f"{node!r} cannot be parsed as a dependency for task {name!r} in {path!r}."
+            f"{node!r} cannot be parsed as a {kind} for task {name!r} in {path!r}."
         )
 
     return collected_node
@@ -462,7 +481,7 @@ def _collect_dependencies(
     node = node_info.value
 
     collected_node = session.hook.pytask_collect_node(
-        session=session, path=path, node_info=node_info, node=node
+        session=session, path=path, node_info=node_info
     )
     if collected_node is None:
         raise NodeNotCollectedError(
@@ -509,9 +528,10 @@ def _collect_product(
 
     if isinstance(node, str):
         node = Path(node)
+        node_info = node_info._replace(value=node)
 
     collected_node = session.hook.pytask_collect_node(
-        session=session, path=path, node_info=node_info, node=node
+        session=session, path=path, node_info=node_info
     )
     if collected_node is None:
         raise NodeNotCollectedError(

src/_pytask/task_utils.py

@@ -114,15 +114,6 @@ def parse_collected_tasks_with_task_marker(
         else:
             collected_tasks[name] = [i[1] for i in parsed_tasks if i[0] == name][0]
 
-    # TODO: Remove when parsing dependencies and products from all arguments is
-    # implemented.
-    for task in collected_tasks.values():
-        meta = task.pytask_meta  # type: ignore[attr-defined]
-        for marker_name in ("depends_on", "produces"):
-            if marker_name in meta.kwargs:
-                value = meta.kwargs.pop(marker_name)
-                meta.markers.append(Mark(marker_name, (value,), {}))
-
     return collected_tasks
 
 

tests/test_collect.py

@@ -8,6 +8,7 @@
 import pytest
 from _pytask.collect import _find_shortest_uniquely_identifiable_name_for_tasks
 from _pytask.collect import pytask_collect_node
+from _pytask.exceptions import NodeNotCollectedError
 from _pytask.models import NodeInfo
 from pytask import cli
 from pytask import CollectionOutcome
@@ -53,7 +54,7 @@ def task_with_non_path_dependency():
     assert session.exit_code == ExitCode.COLLECTION_FAILED
     assert session.collection_reports[0].outcome == CollectionOutcome.FAIL
     exc_info = session.collection_reports[0].exc_info
-    assert isinstance(exc_info[1], ValueError)
+    assert isinstance(exc_info[1], NodeNotCollectedError)
     assert "'@pytask.mark.depends_on'" in str(exc_info[1])
 
 
@@ -74,7 +75,7 @@ def task_with_non_path_dependency():
     assert session.exit_code == ExitCode.COLLECTION_FAILED
     assert session.collection_reports[0].outcome == CollectionOutcome.FAIL
     exc_info = session.collection_reports[0].exc_info
-    assert isinstance(exc_info[1], ValueError)
+    assert isinstance(exc_info[1], NodeNotCollectedError)
     assert "'@pytask.mark.depends_on'" in str(exc_info[1])
 
 
@@ -326,7 +327,7 @@ def task_write_text(produces="out.txt"):
 
 
 @pytest.mark.end_to_end()
-def test_collect_string_product_as_function_default_fails(tmp_path):
+def test_collect_string_product_as_function_default(tmp_path):
     source = """
     import pytask
 
@@ -335,9 +336,8 @@ def task_write_text(produces="out.txt"):
     """
     tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source))
     session = main({"paths": tmp_path})
-    report = session.collection_reports[0]
-    assert report.outcome == CollectionOutcome.FAIL
-    assert "If you use 'produces'" in str(report.exc_info[1])
+    assert session.exit_code == ExitCode.OK
+    assert tmp_path.joinpath("out.txt").exists()
 
 
 @pytest.mark.end_to_end()

tests/test_task.py

@@ -478,7 +478,7 @@ def task_example(
         path_out.write_text(path_in.read_text() + " " + arg)
     """
     tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source))
-    tmp_path.joinpath("input.txt").write_text("hello")
+    tmp_path.joinpath("input.txt").write_text("Hello")
 
     result = runner.invoke(cli, [tmp_path.as_posix()])
     assert result.exit_code == ExitCode.OK