Merge c34b17e into 7885402

Author: tobiasraabe

docs/source/changes.md

@@ -17,6 +17,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and
 - {pull}`489` simplifies parsing products and does not raise an error when a product
   annotation is used with the argument name `produces`. And, allow `produces` to intake
   any node.
+- {pull}`490` refactors and better tests parsing of dependencies.
 
 ## 0.4.2 - 2023-11-8
 

src/_pytask/collect_utils.py

@@ -267,21 +267,6 @@ def parse_dependencies_from_task_function(
     # Parse dependencies from task when @task is used.
     if "depends_on" in kwargs:
         has_depends_on_argument = True
-        dependencies["depends_on"] = tree_map(
-            lambda x: _collect_decorator_node(
-                session,
-                node_path,
-                task_name,
-                NodeInfo(
-                    arg_name="depends_on",
-                    path=(),
-                    value=x,
-                    task_path=task_path,
-                    task_name=task_name,
-                ),
-            ),
-            kwargs["depends_on"],
-        )
 
     if has_depends_on_decorator and has_depends_on_argument:
         raise NodeNotCollectedError(_ERROR_MULTIPLE_DEPENDENCY_DEFINITIONS)
@@ -308,9 +293,6 @@ def parse_dependencies_from_task_function(
         ):
             continue
 
-        if parameter_name == "depends_on":
-            continue
-
         nodes = tree_map_with_path(
             lambda p, x: _collect_dependency(
                 session,
@@ -461,7 +443,6 @@ def parse_products_from_task_function(  # noqa: C901
                         task_path=task_path,
                         task_name=task_name,
                     ),
-                    convert_string_to_path=parameter_name == "produces",  # noqa: B023
                 ),
                 value,
             )
@@ -481,7 +462,6 @@ def parse_products_from_task_function(  # noqa: C901
                     task_path=task_path,
                     task_name=task_name,
                 ),
-                convert_string_to_path=False,
             ),
             parameters_with_node_annot["return"],
         )
@@ -502,7 +482,6 @@ def parse_products_from_task_function(  # noqa: C901
                     task_path=task_path,
                     task_name=task_name,
                 ),
-                convert_string_to_path=False,
             ),
             task_produces,
         )
@@ -606,6 +585,16 @@ def _collect_dependency(
     """
     node = node_info.value
 
+    # If we encounter a string and the argument name is 'depends_on', we convert it.
+    if isinstance(node, str) and node_info.arg_name == "depends_on":
+        warnings.warn(
+            _WARNING_STRING_DEPRECATED.format(kind="dependency", node=node),
+            category=FutureWarning,
+            stacklevel=1,
+        )
+        node = Path(node)
+        node_info = node_info._replace(value=node)
+
     if isinstance(node, PythonNode) and node.value is no_default:
         # If a node is a dependency and its value is not set, the node is a product in
         # another task and the value will be set there. Thus, we wrap the original node
@@ -629,7 +618,6 @@ def _collect_product(
     path: Path,
     task_name: str,
     node_info: NodeInfo,
-    convert_string_to_path: bool = False,
 ) -> PNode:
     """Collect products for a task.
 
@@ -644,8 +632,13 @@ def _collect_product(
     """
     node = node_info.value
 
-    # If we encounter a string and it is allowed, convert it to a path.
-    if isinstance(node, str) and convert_string_to_path:
+    # If we encounter a string and the argument name is 'produces', we convert it.
+    if isinstance(node, str) and node_info.arg_name == "produces":
+        warnings.warn(
+            _WARNING_STRING_DEPRECATED.format(kind="product", node=node),
+            category=FutureWarning,
+            stacklevel=1,
+        )
         node = Path(node)
         node_info = node_info._replace(value=node)
 

tests/test_collect.py

@@ -19,12 +19,20 @@
 
 
 @pytest.mark.end_to_end()
-def test_collect_filepathnode_with_relative_path(tmp_path):
-    source = """
+@pytest.mark.parametrize(
+    ("depends_on", "produces"),
+    [
+        ("'in.txt'", "'out.txt'"),
+        ("Path('in.txt')", "Path('out.txt')"),
+    ],
+)
+def test_collect_file_with_relative_path(tmp_path, depends_on, produces):
+    source = f"""
     import pytask
+    from pathlib import Path
 
-    @pytask.mark.depends_on("in.txt")
-    @pytask.mark.produces("out.txt")
+    @pytask.mark.depends_on({depends_on})
+    @pytask.mark.produces({produces})
     def task_write_text(depends_on, produces):
         produces.write_text(depends_on.read_text())
     """
@@ -37,6 +45,27 @@ def task_write_text(depends_on, produces):
     assert tmp_path.joinpath("out.txt").read_text() == "Relative paths work."
 
 
+@pytest.mark.end_to_end()
+def test_relative_path_of_path_node(runner, tmp_path):
+    source = """
+    from pathlib import Path
+    from typing_extensions import Annotated
+    from pytask import Product, PathNode
+
+    def task_example(
+        in_path: Annotated[Path, PathNode(path=Path("in.txt"))],
+        path: Annotated[Path, Product, PathNode(path=Path("out.txt"))],
+    ) -> None:
+        path.write_text("text")
+    """
+    tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source))
+    tmp_path.joinpath("in.txt").touch()
+
+    result = runner.invoke(cli, [tmp_path.as_posix()])
+    assert result.exit_code == ExitCode.OK
+    assert tmp_path.joinpath("out.txt").exists()
+
+
 @pytest.mark.end_to_end()
 def test_collect_depends_on_that_is_not_str_or_path(capsys, tmp_path):
     """If a node cannot be parsed because unknown type, raise an error."""
@@ -348,30 +377,22 @@ def task_my_task():
 
 
 @pytest.mark.end_to_end()
-def test_collect_string_product_with_task_decorator(runner, tmp_path):
-    source = """
-    import pytask
-
-    @pytask.mark.task
-    def task_write_text(produces="out.txt"):
-        produces.touch()
-    """
-    tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source))
-    result = runner.invoke(cli, [tmp_path.as_posix()])
-    assert result.exit_code == ExitCode.OK
-    assert tmp_path.joinpath("out.txt").exists()
-
+@pytest.mark.parametrize("decorator", ["", "@task"])
+def test_collect_string_product_with_or_without_task_decorator(
+    runner, tmp_path, decorator
+):
+    source = f"""
+    from pytask import task
 
-@pytest.mark.end_to_end()
-def test_collect_string_product_as_function_default(runner, tmp_path):
-    source = """
+    {decorator}
     def task_write_text(produces="out.txt"):
         produces.touch()
     """
     tmp_path.joinpath("task_module.py").write_text(textwrap.dedent(source))
     result = runner.invoke(cli, [tmp_path.as_posix()])
     assert result.exit_code == ExitCode.OK
     assert tmp_path.joinpath("out.txt").exists()
+    assert "FutureWarning" in result.output
 
 
 @pytest.mark.end_to_end()
@@ -443,12 +464,19 @@ def task_example(
 
 
 @pytest.mark.end_to_end()
-def test_deprecation_warning_for_strings_in_depends_on(runner, tmp_path):
-    source = """
+@pytest.mark.parametrize(
+    ("depends_on", "produces"),
+    [("'in.txt'", "Path('out.txt')"), ("Path('in.txt')", "'out.txt'")],
+)
+def test_deprecation_warning_for_strings_in_former_decorator_args(
+    runner, tmp_path, depends_on, produces
+):
+    source = f"""
     import pytask
+    from pathlib import Path
 
-    @pytask.mark.depends_on("in.txt")
-    @pytask.mark.produces("out.txt")
+    @pytask.mark.depends_on({depends_on})
+    @pytask.mark.produces({produces})
     def task_write_text(depends_on, produces):
         produces.touch()
     """
@@ -472,7 +500,6 @@ def task_write_text(depends_on, produces=Path("out.txt")):
 
     result = runner.invoke(cli, [tmp_path.as_posix()])
     assert "FutureWarning" not in result.output
-    assert "Using 'produces' as an argument name" not in result.output
 
 
 @pytest.mark.end_to_end()
@@ -537,26 +564,6 @@ def task_example(path: Annotated[Path, Product, node]) -> None:
     assert "ValueError: The value for the parameter 'path'" in result.output
 
 
-@pytest.mark.end_to_end()
-def test_relative_path_of_path_node(runner, tmp_path):
-    source = """
-    from pathlib import Path
-    from typing_extensions import Annotated
-    from pytask import Product, PathNode
-
-    def task_example(
-        path: Annotated[Path, Product, PathNode(path=Path("out.txt"), name="product")],
-    ) -> None:
-        path.write_text("text")
-    """
-    tmp_path.joinpath("subfolder").mkdir()
-    tmp_path.joinpath("subfolder", "task_module.py").write_text(textwrap.dedent(source))
-
-    result = runner.invoke(cli, [tmp_path.as_posix()])
-    assert result.exit_code == ExitCode.OK
-    assert tmp_path.joinpath("subfolder", "out.txt").exists()
-
-
 @pytest.mark.end_to_end()
 def test_error_when_using_kwargs_and_node_in_annotation(runner, tmp_path):
     source = """