Reduce node names in resolve dependency errors. (#47)

Author: tobiasraabe

docs/changes.rst

@@ -15,6 +15,7 @@ all releases are available on `Anaconda.org <https://anaconda.org/pytask/pytask>
   input for ``@pytask.mark.depends_on`` and ``@pytask.mark.produces`` are preserved as a
   dictionary inside the function.
 - :gh:`43` releases v0.0.10
+- :gh:`47` reduce node names in error messages while resolving dependencies.
 
 
 0.0.9 - 2020-10-28

environment.yml

@@ -28,6 +28,7 @@ dependencies:
   - pydot
   - pytest-cov
   - tox-conda
+  - virtualenv=20.0.33
 
   # Documentation
   - nbsphinx

src/_pytask/collect.py

@@ -13,7 +13,7 @@
 from _pytask.mark import has_marker
 from _pytask.nodes import FilePathNode
 from _pytask.nodes import PythonFunctionTask
-from _pytask.nodes import shorten_node_name
+from _pytask.nodes import reduce_node_name
 from _pytask.report import CollectionReport
 from _pytask.report import format_collect_footer
 
@@ -234,8 +234,8 @@ def pytask_collect_log(session, reports, tasks):
             if report.node is None:
                 header = " Error "
             else:
-                shortened_name = shorten_node_name(report.node, session.config["paths"])
-                header = f" Could not collect {shortened_name} "
+                short_name = reduce_node_name(report.node, session.config["paths"])
+                header = f" Could not collect {short_name} "
 
             click.echo(f"{{:_^{tm_width}}}".format(header))
 

src/_pytask/execute.py

@@ -13,7 +13,7 @@
 from _pytask.exceptions import NodeNotFoundError
 from _pytask.mark import Mark
 from _pytask.nodes import FilePathNode
-from _pytask.nodes import shorten_node_name
+from _pytask.nodes import reduce_node_name
 from _pytask.report import ExecutionReport
 from _pytask.report import format_execute_footer
 
@@ -168,7 +168,7 @@ def pytask_execute_log_end(session, reports):
     for report in reports:
         if not report.success:
 
-            task_name = shorten_node_name(report.task, session.config["paths"])
+            task_name = reduce_node_name(report.task, session.config["paths"])
             message = f" Task {task_name} failed "
             if len(message) > tm_width:
                 click.echo("_" * tm_width)

src/_pytask/nodes.py

@@ -355,6 +355,11 @@ def _relative_to(path: Path, source: Path, include_source: bool = True):
 def _find_closest_ancestor(path: Path, potential_ancestors: List[Path]):
     """Find the closest ancestor of a path.
 
+    In case only a single path to a task file is passed, we take the parent folder of
+    this file. The check :meth:`pathlib.Path.is_file` only succeeds when the file
+    exists. This must be true as otherwise an error is raised by :obj:`click` right in
+    the beginning.
+
     Examples
     --------
     >>> from pathlib import Path
@@ -371,6 +376,11 @@ def _find_closest_ancestor(path: Path, potential_ancestors: List[Path]):
         if ancestor == path:
             closest_ancestor = path
             break
+
+        # Paths can also point to files in which case we want to take the parent folder.
+        if ancestor.is_file():
+            ancestor = ancestor.parent
+
         if ancestor in path.parents:
             if closest_ancestor is None or (
                 len(path.relative_to(ancestor).parts)
@@ -381,13 +391,13 @@ def _find_closest_ancestor(path: Path, potential_ancestors: List[Path]):
     return closest_ancestor
 
 
-def shorten_node_name(node, paths: List[Path]):
-    """Shorten the node name.
+def reduce_node_name(node, paths: List[Path]):
+    """Reduce the node name.
 
     The whole name of the node - which includes the drive letter - can be very long
     when using nested folder structures in bigger projects.
 
-    Thus, the part of the name which contains the path is replace by the relative
+    Thus, the part of the name which contains the path is replaced by the relative
     path from one path in ``session.config["paths"]`` to the node.
 
     """

src/_pytask/resolve_dependencies.py

@@ -12,6 +12,7 @@
 from _pytask.exceptions import NodeNotFoundError
 from _pytask.exceptions import ResolvingDependenciesError
 from _pytask.mark import Mark
+from _pytask.nodes import reduce_node_name
 from _pytask.report import ResolvingDependenciesReport
 from _pytask.traceback import remove_traceback_from_exc_info
 from pony import orm
@@ -86,11 +87,11 @@ def pytask_resolve_dependencies_select_execution_dag(dag):
 
 
 @hookimpl
-def pytask_resolve_dependencies_validate_dag(dag):
+def pytask_resolve_dependencies_validate_dag(session, dag):
     """Validate the DAG."""
     _check_if_dag_has_cycles(dag)
-    _check_if_root_nodes_are_available(dag)
-    _check_if_tasks_have_the_same_products(dag)
+    _check_if_root_nodes_are_available(dag, session)
+    _check_if_tasks_have_the_same_products(dag, session)
 
 
 def _have_task_or_neighbors_changed(task_name, dag):
@@ -132,7 +133,8 @@ def _check_if_dag_has_cycles(dag):
         )
 
 
-def _check_if_root_nodes_are_available(dag):
+def _check_if_root_nodes_are_available(dag, session):
+    paths = session.config["paths"]
     missing_root_nodes = {}
 
     for node in dag.nodes:
@@ -142,7 +144,12 @@ def _check_if_root_nodes_are_available(dag):
             try:
                 dag.nodes[node]["node"].state()
             except NodeNotFoundError:
-                missing_root_nodes[node] = list(dag.successors(node))
+                # Shorten node names for better printing.
+                short_node_name = reduce_node_name(dag.nodes[node]["node"], paths)
+                short_successors = _reduce_names_of_multiple_nodes(
+                    dag.successors(node), dag, paths
+                )
+                missing_root_nodes[short_node_name] = short_successors
 
     if missing_root_nodes:
         raise ResolvingDependenciesError(
@@ -153,15 +160,19 @@ def _check_if_root_nodes_are_available(dag):
         )
 
 
-def _check_if_tasks_have_the_same_products(dag):
+def _check_if_tasks_have_the_same_products(dag, session):
+    paths = session.config["paths"]
     nodes_created_by_multiple_tasks = {}
 
     for node in dag.nodes:
         is_node = "node" in dag.nodes[node]
         if is_node:
             parents = list(dag.predecessors(node))
             if len(parents) > 1:
-                nodes_created_by_multiple_tasks[node] = parents
+                # Reduce node names for better printing.
+                short_node = reduce_node_name(dag.nodes[node]["node"], paths)
+                short_parents = _reduce_names_of_multiple_nodes(parents, dag, paths)
+                nodes_created_by_multiple_tasks[short_node] = short_parents
 
     if nodes_created_by_multiple_tasks:
         raise ResolvingDependenciesError(
@@ -184,3 +195,11 @@ def pytask_resolve_dependencies_log(session, report):
 
     click.echo("")
     click.echo("=" * tm_width)
+
+
+def _reduce_names_of_multiple_nodes(names, dag, paths):
+    """Reduce the names of multiple nodes in the DAG."""
+    return [
+        reduce_node_name(dag.nodes[n].get("node") or dag.nodes[n].get("task"), paths)
+        for n in names
+    ]

tests/test_nodes.py

@@ -17,7 +17,7 @@
 from _pytask.nodes import MetaNode
 from _pytask.nodes import MetaTask
 from _pytask.nodes import produces
-from _pytask.nodes import shorten_node_name
+from _pytask.nodes import reduce_node_name
 
 
 @pytest.mark.unit
@@ -196,10 +196,13 @@ def test_relative_to(path, source, include_source, expected):
     [
         (Path("src/task.py"), [Path("src"), Path("bld")], Path("src")),
         (Path("tasks/task.py"), [Path("src"), Path("bld")], None),
-        (Path("src/tasks/task.py"), [Path("src"), Path("src/tasks")], Path("tasks")),
+        (Path("src/ts/task.py"), [Path("src"), Path("src/ts")], Path("src/ts")),
+        (Path("src/in.txt"), [Path("src/task_d.py")], Path("src")),
     ],
 )
-def task_find_closest_ancestor(path, potential_ancestors, expected):
+def test_find_closest_ancestor(monkeypatch, path, potential_ancestors, expected):
+    # Ensures that files are detected by an existing suffix not if they also exist.
+    monkeypatch.setattr("_pytask.nodes.pathlib.Path.is_file", lambda x: bool(x.suffix))
     result = _find_closest_ancestor(path, potential_ancestors)
     assert result == expected
 
@@ -259,9 +262,9 @@ class FalseNode:
         ),
     ],
 )
-def test_shorten_node_name(node, paths, expectation, expected):
+def test_reduce_node_name(node, paths, expectation, expected):
     with expectation:
-        result = shorten_node_name(node, paths)
+        result = reduce_node_name(node, paths)
         assert result == expected
 
 

tests/test_resolve_dependencies.py

@@ -1,5 +1,6 @@
 import textwrap
 from contextlib import ExitStack as does_not_raise  # noqa: N813
+from pathlib import Path
 
 import attr
 import networkx as nx
@@ -37,6 +38,10 @@ def state(self):
             raise NodeNotFoundError
 
 
+class DummySession:
+    pass
+
+
 @pytest.mark.unit
 def test_create_dag():
     task = Task(
@@ -53,22 +58,30 @@ def test_create_dag():
 def test_check_if_root_nodes_are_available():
     dag = nx.DiGraph()
 
+    root = Path("directory")
+    session = DummySession()
+    session.config = {"paths": [root]}
+
     task = Task("task")
+    task.path = root.joinpath("task_dummy")
+    task.base_name = "task_dummy"
     dag.add_node(task.name, task=task)
 
     available_node = Node("available")
+    available_node.path = root.joinpath("available_node")
     dag.add_node(available_node.name, node=available_node)
     dag.add_edge(available_node.name, task.name)
 
     with does_not_raise():
-        _check_if_root_nodes_are_available(dag)
+        _check_if_root_nodes_are_available(dag, session)
 
     missing_node = Node("missing")
+    missing_node.path = root.joinpath("missing_node")
     dag.add_node(missing_node.name, node=missing_node)
     dag.add_edge(missing_node.name, task.name)
 
     with pytest.raises(ResolvingDependenciesError):
-        _check_if_root_nodes_are_available(dag)
+        _check_if_root_nodes_are_available(dag, session)
 
 
 @pytest.mark.end_to_end
@@ -78,16 +91,24 @@ def test_check_if_root_nodes_are_available_end_to_end(tmp_path, runner):
 
     @pytask.mark.depends_on("in.txt")
     @pytask.mark.produces("out.txt")
-    def task_dummy(produces):
+    def task_d(produces):
         produces.write_text("1")
     """
-    tmp_path.joinpath("task_dummy.py").write_text(textwrap.dedent(source))
+    tmp_path.joinpath("task_d.py").write_text(textwrap.dedent(source))
 
     result = runner.invoke(cli, [tmp_path.as_posix()])
 
     assert result.exit_code == 4
     assert "Failures during resolving dependencies" in result.output
 
+    # Ensure that node names are reduced.
+    assert "Failures during resolving dependencies" in result.output
+    assert "There are some dependencies missing which do not" in result.output
+    assert tmp_path.joinpath("task_d.py").as_posix() + "::task_d" not in result.output
+    assert tmp_path.name + "/task_d.py::task_d" in result.output
+    assert tmp_path.joinpath("in.txt").as_posix() not in result.output
+    assert tmp_path.name + "/in.txt" in result.output
+
 
 @pytest.mark.end_to_end
 def test_cycle_in_dag(tmp_path, runner):
@@ -110,6 +131,7 @@ def task_2(produces):
 
     assert result.exit_code == 4
     assert "Failures during resolving dependencies" in result.output
+    assert "The DAG contains cycles which means a dependency" in result.output
 
 
 @pytest.mark.end_to_end
@@ -125,9 +147,18 @@ def task_1(produces):
     def task_2(produces):
         produces.write_text("2")
     """
-    tmp_path.joinpath("task_dummy.py").write_text(textwrap.dedent(source))
+    tmp_path.joinpath("task_d.py").write_text(textwrap.dedent(source))
 
     result = runner.invoke(cli, [tmp_path.as_posix()])
 
     assert result.exit_code == 4
     assert "Failures during resolving dependencies" in result.output
+    assert "There are some tasks which produce the same output." in result.output
+
+    # Ensure that nodes names are reduced.
+    assert tmp_path.joinpath("task_d.py").as_posix() + "::task_1" not in result.output
+    assert tmp_path.name + "/task_d.py::task_1" in result.output
+    assert tmp_path.joinpath("task_d.py").as_posix() + "::task_2" not in result.output
+    assert tmp_path.name + "/task_d.py::task_2" in result.output
+    assert tmp_path.joinpath("out.txt").as_posix() not in result.output
+    assert tmp_path.name + "/out.txt" in result.output