diff --git a/docs/source/changes.md b/docs/source/changes.md index 78f89b0a..90cfec84 100644 --- a/docs/source/changes.md +++ b/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 diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 89d51376..6f306e02 100644 --- a/src/_pytask/collect_utils.py +++ b/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) diff --git a/tests/test_collect.py b/tests/test_collect.py index ce23d8f0..e17622b5 100644 --- a/tests/test_collect.py +++ b/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,23 +377,14 @@ 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() """ @@ -372,6 +392,7 @@ def task_write_text(produces="out.txt"): 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 = """