Fix ids of PythonNodes. (#433)

Author: tobiasraabe

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
 

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)
 
 

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
 
@@ -20,6 +21,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
@@ -34,7 +36,6 @@
 
 
 if TYPE_CHECKING:
-    from pathlib import Path
     from typing import NoReturn
 
 
@@ -155,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,
@@ -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,18 @@ def _print_collected_tasks(
                         )
                         text = Text(reduced_node_name, style=url_style)
                     else:
-                        text = node.name  # type: ignore[attr-defined]
+                        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, "<Dependency ", text, ">"))
 
-                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 +241,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, "<Product ", text, ">"))
 
     console.print(tree)

src/_pytask/collect_utils.py

@@ -71,16 +71,30 @@ 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__
     objects = _extract_nodes_from_function_markers(obj, parser)
     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,
         )
@@ -297,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
@@ -352,7 +384,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 +404,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 +420,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 +450,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 +463,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 +483,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 +504,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,

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

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)