From 297d49cc8a12ad43e20bbe96c1bcfbcc90a8be3e Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 14:51:55 +0200 Subject: [PATCH 1/7] add some little fixes. --- src/_pytask/collect_command.py | 13 +++++++------ tox.ini | 7 ------- 2 files changed, 7 insertions(+), 13 deletions(-) diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index 7f7de6c3..1124c5c2 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -20,6 +20,7 @@ from _pytask.exceptions import ResolvingDependenciesError from _pytask.mark import select_by_keyword from _pytask.mark import select_by_mark +from _pytask.node_protocols import PNode from _pytask.node_protocols import PPathNode from _pytask.node_protocols import PTask from _pytask.node_protocols import PTaskWithPath @@ -199,10 +200,10 @@ def _print_collected_tasks( ) if show_nodes: - nodes = list(tree_leaves(task.depends_on)) - sorted_nodes = sorted( - nodes, key=lambda x: x.name # type: ignore[attr-defined] + nodes: list[PNode] = list( + tree_leaves(task.depends_on) # type: ignore[arg-type] ) + sorted_nodes = sorted(nodes, key=lambda x: x.name) for node in sorted_nodes: if isinstance(node, PPathNode): if node.path.as_posix() in node.name: @@ -216,11 +217,11 @@ def _print_collected_tasks( ) text = Text(reduced_node_name, style=url_style) else: - text = node.name # type: ignore[attr-defined] + text = node.name task_branch.add(Text.assemble(FILE_ICON, "")) - for node in sorted( + for node in sorted( # type: ignore[assignment] tree_leaves(task.produces), key=lambda x: getattr( x, "path", x.name # type: ignore[attr-defined] @@ -233,7 +234,7 @@ def _print_collected_tasks( ) text = Text(reduced_node_name, style=url_style) else: - text = Text(node.name) # type: ignore[attr-defined] + text = Text(node.name) task_branch.add(Text.assemble(FILE_ICON, "")) console.print(tree) diff --git a/tox.ini b/tox.ini index 82a9d52b..5bb79ff1 100644 --- a/tox.ini +++ b/tox.ini @@ -33,10 +33,3 @@ conda_deps = commands = pytest {posargs} - -[testenv:sphinx] -changedir = docs/source -conda_env = docs/rtd_environment.yml -commands = - sphinx-build -T -b html -d {envtmpdir}/doctrees . {envtmpdir}/html - - sphinx-build -T -b linkcheck -d {envtmpdir}/doctrees . {envtmpdir}/linkcheck From acbddfc4a39b0c3187083479a8a1e89e2ea2fd81 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 15:59:05 +0200 Subject: [PATCH 2/7] Add failing test. --- tests/test_dag.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_dag.py b/tests/test_dag.py index d3b96303..f233c26f 100644 --- a/tests/test_dag.py +++ b/tests/test_dag.py @@ -6,6 +6,7 @@ import pytest from _pytask.dag import pytask_dag_create_dag from attrs import define +from pytask import build from pytask import cli from pytask import ExitCode from pytask import NodeNotFoundError @@ -212,3 +213,15 @@ def task_example(a = PythonNode(value={"a": 1}, hash=True)): result = runner.invoke(cli, [tmp_path.as_posix()]) assert result.exit_code == ExitCode.DAG_FAILED assert "task_example" in result.output + + +def test_python_nodes_are_unique(tmp_path): + tmp_path.joinpath("a").mkdir() + tmp_path.joinpath("a", "task_example.py").write_text("def task_example(a=1): pass") + tmp_path.joinpath("b").mkdir() + tmp_path.joinpath("b", "task_example.py").write_text("def task_example(a=2): pass") + + session = build(paths=tmp_path) + + assert session.exit_code == ExitCode.OK + assert len(session.dag.nodes) == 4 From cd5ec832ef40306eeb95d8c50e3fded36234ecb9 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 16:00:43 +0200 Subject: [PATCH 3/7] Fix. --- src/_pytask/collect.py | 34 +++++++++--- src/_pytask/collect_utils.py | 103 ++++++++++++++++++++++++++--------- src/_pytask/models.py | 3 + src/_pytask/report.py | 1 - tests/test_collect.py | 26 ++++++++- 5 files changed, 131 insertions(+), 36 deletions(-) diff --git a/src/_pytask/collect.py b/src/_pytask/collect.py index 95b43e29..fa5e9139 100644 --- a/src/_pytask/collect.py +++ b/src/_pytask/collect.py @@ -231,9 +231,11 @@ def pytask_collect_task( if (name.startswith("task_") or has_mark(obj, "task")) and callable(obj): path_nodes = Path.cwd() if path is None else path.parent dependencies = parse_dependencies_from_task_function( - session, path_nodes, name, obj + session, path, name, path_nodes, obj + ) + products = parse_products_from_task_function( + session, path, name, path_nodes, obj ) - products = parse_products_from_task_function(session, path_nodes, name, obj) markers = obj.pytask_meta.markers if hasattr(obj, "pytask_meta") else [] @@ -306,9 +308,20 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PN node = node_info.value if isinstance(node, PythonNode): - if not node.name: - suffix = "-" + "-".join(map(str, node_info.path)) if node_info.path else "" - node.name = node_info.arg_name + suffix + prefix = ( + node_info.task_path.as_posix() + "::" + node_info.task_name + if node_info.task_path + else node_info.task_name + ) + if node.name: + node.name = prefix + "::" + node.name + else: + node.name = prefix + "::" + node_info.arg_name + + suffix = "-".join(map(str, node_info.path)) if node_info.path else "" + if suffix: + node.name += "::" + suffix + return node if isinstance(node, PPathNode) and not node.path.is_absolute(): @@ -336,8 +349,15 @@ def pytask_collect_node(session: Session, path: Path, node_info: NodeInfo) -> PN ) return PathNode.from_path(node) - suffix = "-" + "-".join(map(str, node_info.path)) if node_info.path else "" - node_name = node_info.arg_name + suffix + prefix = ( + node_info.task_path.as_posix() + "::" + node_info.task_name + if node_info.task_path + else node_info.task_name + ) + node_name = prefix + "::" + node_info.arg_name + suffix = "-".join(map(str, node_info.path)) if node_info.path else "" + if suffix: + node_name += "::" + suffix return PythonNode(value=node, name=node_name) diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index 4c47c178..d9acd805 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -71,8 +71,13 @@ def produces(objects: PyTree[Any]) -> PyTree[Any]: return objects -def parse_nodes( - session: Session, path: Path, name: str, obj: Any, parser: Callable[..., Any] +def parse_nodes( # noqa: PLR0913 + session: Session, + task_path: Path, + task_name: str, + node_path: Path, + obj: Any, + parser: Callable[..., Any], ) -> Any: """Parse nodes from object.""" arg_name = parser.__name__ @@ -80,7 +85,16 @@ def parse_nodes( nodes = _convert_objects_to_node_dictionary(objects, arg_name) return tree_map( lambda x: _collect_decorator_node( - session, path, name, NodeInfo(arg_name, (), x) + session, + node_path, + task_name, + NodeInfo( + arg_name=arg_name, + path=(), + value=x, + task_path=task_path, + task_name=task_name, + ), ), nodes, ) @@ -226,7 +240,7 @@ def _merge_dictionaries(list_of_dicts: list[dict[Any, Any]]) -> dict[Any, Any]: def parse_dependencies_from_task_function( - session: Session, path: Path, name: str, obj: Any + session: Session, task_path: Path, task_name: str, node_path: Path, obj: Any ) -> dict[str, Any]: """Parse dependencies from task function.""" has_depends_on_decorator = False @@ -235,7 +249,7 @@ def parse_dependencies_from_task_function( if has_mark(obj, "depends_on"): has_depends_on_decorator = True - nodes = parse_nodes(session, path, name, obj, depends_on) + nodes = parse_nodes(session, task_path, task_name, node_path, obj, depends_on) dependencies["depends_on"] = nodes task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} @@ -248,7 +262,16 @@ def parse_dependencies_from_task_function( has_depends_on_argument = True dependencies["depends_on"] = tree_map( lambda x: _collect_decorator_node( - session, path, name, NodeInfo(arg_name="depends_on", path=(), value=x) + session, + node_path, + task_name, + NodeInfo( + arg_name="depends_on", + path=(), + value=x, + task_path=task_path, + task_name=task_name, + ), ), kwargs["depends_on"], ) @@ -284,9 +307,15 @@ def parse_dependencies_from_task_function( nodes = tree_map_with_path( lambda p, x: _collect_dependency( session, - path, - name, - NodeInfo(parameter_name, p, x), # noqa: B023 + node_path, + task_name, + NodeInfo( + arg_name=parameter_name, # noqa: B023 + path=p, + value=x, + task_path=task_path, + task_name=task_name, + ), ), value, ) @@ -352,7 +381,7 @@ def task_example(produces: Annotated[..., Product]): def parse_products_from_task_function( - session: Session, path: Path, name: str, obj: Any + session: Session, task_path: Path, task_name: str, node_path: Path, obj: Any ) -> dict[str, Any]: """Parse products from task function. @@ -372,7 +401,7 @@ def parse_products_from_task_function( # Parse products from decorators. if has_mark(obj, "produces"): has_produces_decorator = True - nodes = parse_nodes(session, path, name, obj, produces) + nodes = parse_nodes(session, task_path, task_name, node_path, obj, produces) out = {"produces": nodes} task_kwargs = obj.pytask_meta.kwargs if hasattr(obj, "pytask_meta") else {} @@ -388,9 +417,15 @@ def parse_products_from_task_function( collected_products = tree_map_with_path( lambda p, x: _collect_product( session, - path, - name, - NodeInfo(arg_name="produces", path=p, value=x), + node_path, + task_name, + NodeInfo( + arg_name="produces", + path=p, + value=x, + task_path=task_path, + task_name=task_name, + ), is_string_allowed=True, ), kwargs["produces"], @@ -412,8 +447,8 @@ def parse_products_from_task_function( and parameter_name in parameters_with_node_annot ): msg = ( - f"The value for the parameter {name!r} is defined twice in " - "'@pytask.mark.task(kwargs=...)' and in the type annotation. " + f"The value for the parameter {parameter_name!r} is defined twice " + "in '@pytask.mark.task(kwargs=...)' and in the type annotation. " "Choose only one option." ) raise ValueError(msg) @@ -425,9 +460,15 @@ def parse_products_from_task_function( collected_products = tree_map_with_path( lambda p, x: _collect_product( session, - path, - name, - NodeInfo(parameter_name, p, x), # noqa: B023 + node_path, + task_name, + NodeInfo( + arg_name=parameter_name, # noqa: B023 + path=p, + value=x, + task_path=task_path, + task_name=task_name, + ), is_string_allowed=False, ), value, @@ -439,9 +480,15 @@ def parse_products_from_task_function( collected_products = tree_map_with_path( lambda p, x: _collect_product( session, - path, - name, - NodeInfo("return", p, x), + node_path, + task_name, + NodeInfo( + arg_name="return", + path=p, + value=x, + task_path=task_path, + task_name=task_name, + ), is_string_allowed=False, ), parameters_with_node_annot["return"], @@ -454,9 +501,15 @@ def parse_products_from_task_function( collected_products = tree_map_with_path( lambda p, x: _collect_product( session, - path, - name, - NodeInfo("return", p, x), + node_path, + task_name, + NodeInfo( + arg_name="return", + path=p, + value=x, + task_path=task_path, + task_name=task_name, + ), is_string_allowed=False, ), task_produces, diff --git a/src/_pytask/models.py b/src/_pytask/models.py index 7d442783..ee26ef7e 100644 --- a/src/_pytask/models.py +++ b/src/_pytask/models.py @@ -9,6 +9,7 @@ from attrs import field if TYPE_CHECKING: + from pathlib import Path from _pytask.tree_util import PyTree from _pytask.mark import Mark @@ -33,3 +34,5 @@ class NodeInfo(NamedTuple): arg_name: str path: tuple[str | int, ...] value: Any + task_path: Path + task_name: str diff --git a/src/_pytask/report.py b/src/_pytask/report.py index c29cdec4..74e524ec 100644 --- a/src/_pytask/report.py +++ b/src/_pytask/report.py @@ -37,7 +37,6 @@ def from_exception( exc_info: ExceptionInfo, node: MetaNode | None = None, ) -> CollectionReport: - exc_info = remove_internal_traceback_frames_from_exc_info(exc_info) return cls(outcome=outcome, node=node, exc_info=exc_info) diff --git a/tests/test_collect.py b/tests/test_collect.py index 93325e6f..75533914 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -154,14 +154,26 @@ def test_collect_files_w_custom_file_name_pattern( pytest.param( Session({"check_casing_of_paths": False}, None), Path(), - NodeInfo("", (), Path.cwd() / "text.txt"), + NodeInfo( + arg_name="", + path=(), + value=Path.cwd() / "text.txt", + task_path=Path.cwd() / "task_example.py", + task_name="task_example", + ), Path.cwd() / "text.txt", id="test with absolute string path", ), pytest.param( Session({"check_casing_of_paths": False}, None), Path(), - NodeInfo("", (), 1), + NodeInfo( + arg_name="", + path=(), + value=1, + task_path=Path.cwd() / "task_example.py", + task_name="task_example", + ), "1", id="test with python node", ), @@ -203,7 +215,15 @@ def test_pytask_collect_node_does_not_raise_error_if_path_is_not_normalized( with warnings.catch_warnings(record=True) as record: result = pytask_collect_node( - session, tmp_path, NodeInfo("", (), collected_node) + session, + tmp_path, + NodeInfo( + arg_name="", + path=(), + value=collected_node, + task_path=tmp_path / "task_example.py", + task_name="task_example", + ), ) assert not record From 9d9b84b73e8a3add2e5568e876d743809f3f235e Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 16:42:27 +0200 Subject: [PATCH 4/7] Fix display. --- src/_pytask/collect_command.py | 8 ++++++-- tests/test_collect_command.py | 4 ++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index 1124c5c2..86649071 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -3,6 +3,7 @@ import sys from collections import defaultdict +from pathlib import Path from typing import Any from typing import TYPE_CHECKING @@ -35,7 +36,6 @@ if TYPE_CHECKING: - from pathlib import Path from typing import NoReturn @@ -217,7 +217,11 @@ def _print_collected_tasks( ) text = Text(reduced_node_name, style=url_style) else: - text = node.name + path_part, rest = node.name.split("::", maxsplit=1) + reduced_path = str( + relative_to(Path(path_part), common_ancestor) + ) + text = reduced_path + "::" + rest task_branch.add(Text.assemble(FILE_ICON, "")) diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index 86a48386..24d696b9 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -457,7 +457,7 @@ def task_example( assert "task_module.py>" in captured assert "" in captured - assert "" in result.output + assert "task_example::dependency>" in result.output assert "Product" in captured @@ -482,7 +482,7 @@ def task_example( assert "task_module.py>" in captured assert "" in captured - assert "" in result.output + assert "task_example::dependency>" in result.output assert "Product" in captured From 1f14f5004fc3ed5165bcec43df6b4b4699399301 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 16:45:09 +0200 Subject: [PATCH 5/7] to changes. --- docs/source/changes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/source/changes.md b/docs/source/changes.md index 8b68eba0..e1aa9adb 100644 --- a/docs/source/changes.md +++ b/docs/source/changes.md @@ -51,6 +51,7 @@ releases are available on [PyPI](https://pypi.org/project/pytask) and - {pull}`430` updates some parts of the documentation. - {pull}`431` enables colors for WSL. - {pull}`432` fixes type checking of `pytask.mark.xxx`. +- {pull}`433` fixes the ids generated for {class}`~pytask.PythonNode`s. ## 0.3.2 - 2023-06-07 From 24d0e9f1568454b3a00c4a52adcbba96737a97e7 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 17:03:45 +0200 Subject: [PATCH 6/7] fix test. --- src/_pytask/collect_command.py | 15 +++++++++------ src/_pytask/collect_utils.py | 5 ++++- tests/test_collect_command.py | 8 ++++---- 3 files changed, 17 insertions(+), 11 deletions(-) diff --git a/src/_pytask/collect_command.py b/src/_pytask/collect_command.py index 86649071..0824dcfe 100644 --- a/src/_pytask/collect_command.py +++ b/src/_pytask/collect_command.py @@ -156,7 +156,7 @@ def _organize_tasks(tasks: list[PTaskWithPath]) -> dict[Path, list[PTaskWithPath return sorted_dict -def _print_collected_tasks( +def _print_collected_tasks( # noqa: PLR0912 dictionary: dict[Path, list[PTaskWithPath]], show_nodes: bool, editor_url_scheme: str, @@ -217,11 +217,14 @@ def _print_collected_tasks( ) text = Text(reduced_node_name, style=url_style) else: - path_part, rest = node.name.split("::", maxsplit=1) - reduced_path = str( - relative_to(Path(path_part), common_ancestor) - ) - text = reduced_path + "::" + rest + try: + path_part, rest = node.name.split("::", maxsplit=1) + reduced_path = str( + relative_to(Path(path_part), common_ancestor) + ) + text = reduced_path + "::" + rest + except Exception: # noqa: BLE001 + text = node.name task_branch.add(Text.assemble(FILE_ICON, "")) diff --git a/src/_pytask/collect_utils.py b/src/_pytask/collect_utils.py index d9acd805..02f05145 100644 --- a/src/_pytask/collect_utils.py +++ b/src/_pytask/collect_utils.py @@ -326,7 +326,10 @@ def parse_dependencies_from_task_function( isinstance(x, PythonNode) and not x.hash for x in tree_leaves(nodes) ) if not isinstance(nodes, PNode) and are_all_nodes_python_nodes_without_hash: - dependencies[parameter_name] = PythonNode(value=value, name=parameter_name) + prefix = task_path.as_posix() + "::" + task_name if task_path else task_name + node_name = prefix + "::" + parameter_name + + dependencies[parameter_name] = PythonNode(value=value, name=node_name) else: dependencies[parameter_name] = nodes return dependencies diff --git a/tests/test_collect_command.py b/tests/test_collect_command.py index 24d696b9..2f15f78e 100644 --- a/tests/test_collect_command.py +++ b/tests/test_collect_command.py @@ -457,7 +457,7 @@ def task_example( assert "task_module.py>" in captured assert "" in captured - assert "task_example::dependency>" in result.output + assert "Dependency" in captured assert "Product" in captured @@ -482,7 +482,7 @@ def task_example( assert "task_module.py>" in captured assert "" in captured - assert "task_example::dependency>" in result.output + assert "Dependency" in result.output assert "Product" in captured @@ -508,7 +508,7 @@ def task_example( assert "task_module.py>" in captured assert "" in captured - assert "" in result.output + assert "nested" in result.output assert "Product" in captured @@ -602,7 +602,7 @@ def task_example( result = runner.invoke(cli, ["collect", "--nodes", tmp_path.as_posix()]) assert result.exit_code == ExitCode.OK - assert "node-name" in result.output + assert "Dependency" in result.output @pytest.mark.end_to_end() From a5a4bebc1882eebae8cebd6136a7300ff3662e44 Mon Sep 17 00:00:00 2001 From: Tobias Raabe Date: Mon, 2 Oct 2023 17:12:46 +0200 Subject: [PATCH 7/7] fix windows. --- tests/test_collect.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/test_collect.py b/tests/test_collect.py index 75533914..b9703002 100644 --- a/tests/test_collect.py +++ b/tests/test_collect.py @@ -198,7 +198,17 @@ def test_pytask_collect_node_raises_error_if_path_is_not_correctly_cased(tmp_pat collected_node = tmp_path / "TeXt.TxT" with pytest.raises(Exception, match="The provided path of"): - pytask_collect_node(session, tmp_path, NodeInfo("", (), collected_node)) + pytask_collect_node( + session, + tmp_path, + NodeInfo( + arg_name="", + path=(), + value=collected_node, + task_path=tmp_path.joinpath("task_example.py"), + task_name="task_example", + ), + ) @pytest.mark.unit()