Repository: kedro-org/kedro
Pull request: Provide useful error message when trying to run specific nodes within namespaces
## Description
<!-- Why was this PR created? -->

This PR addresses [this issue](https://github.com/kedro-org/kedro/issues/1301); currently omitting a namespace when trying to run individual nodes results in an uninformative Value Error:

> ValueError: Pipeline does not contain nodes named ['preprocess_companies_node'].

The changes made in this PR add suggestion to the error description, so that when a user tries to run a node without its namespace the error is more helpful:

> ValueError: Pipeline does not contain nodes named ['preprocess_companies_node']. Did you mean: 
['data_processing.preprocess_companies_node']?

## Development notes
<!-- What have you changed, and how has this been tested? -->

This was tested on the spaceflights starter:
`kedro new --starter=spaceflights`

Expected behaviour:
From the project directory, running `kedro run -n preprocess_companies_node` should yield the following error message:
 ` "ValueError: Pipeline does not contain nodes named ['preprocess_companies_node']. Did you mean: 
['data_processing.preprocess_companies_node']?" `

**Important to note:**
The complexity of the namespace search is _O(n*m)_ with _n_ being the number of nodes in a project and _m_ being the number of arguments provided to `kedro run -n`; for larger pipelines this will slow down. However, this is not the only iteration of the nodes in the pipeline implementation - is the potential slowdown a concern?

## Checklist

- [x] Read the [contributing](https://github.com/kedro-org/kedro/blob/main/CONTRIBUTING.md) guidelines
- [x] Opened this PR as a 'Draft Pull Request' if it is work-in-progress
- [ ] Updated the documentation to reflect the code changes
- [x] Added a description of this change in the [`RELEASE.md`](https://github.com/kedro-org/kedro/blob/main/RELEASE.md) file
- [x] Added tests to cover my changes


<a href="https://gitpod.io/#https://github.com/kedro-org/kedro/pull/1773"><img src="https://gitpod.io/button/open-in-gitpod.svg"/></a>



Problem statement:
[KED-2422] Provide useful error message when trying to run specific nodes within namespaces
(transfer from Jira, created by @datajoely)

### Description
Responded to wizard thread on slack.

The user was trying to run `kedro run -n [node_name]` this was not working for the user since they were trying to run a node that actually lived within a namespace so he actually needed to address it via `modular_pipleine.node_name` 

The error message that the user gets when the node is not found is this one:

```
lib/python3.8/site-packages/kedro/pipeline/pipeline.py:415 

ValueError: Pipeline does not contain nodes named [xxx]
```

Whilst not incorrect, there is no prompt to get the names of the nodes that are in scope to identify if there is (a) A typo (b) what the accepted options will be

As we do more modular pipelines I expect this issue to come up more. Even offering an equivalent of catalog list for nodes would be useful.


If you do the following the namespaces of the specific nodes are not present:

kedro pipeline describe _default_
Nodes:

```
load data (<lambda>) Set Timestep (set_timestep) 
get_batches_time_series (get_batches_time_series) 
load_regressor (load_regressor) 
add_transformers (add_transformers) <Lambda> (<lambda>) plot_all_batches_profiles (plot_all_batches_profiles) 
<Lambda> (<lambda>) 
generate_features (generate_features) 
plot_all_features_overview (plot_all_features_overview) 
Set Batch Type (set_batch_type) 
generate performance report (generate_performance_report) 
```

But to run a particular node you need to specify a namespace:

```
ValueError: Pipeline does not contain nodes named ['generate_performance_report'].
```

Instead you have to specify:

```
kedro run --node train.train_model.generate_performance_report
```

### Proposed solution
So can we include the namespaces in the ValueError?

Hints from issue discussion:
We could also say the node name `'generate_performance_report' is available under the following namespaces ['a', 'b', 'c'], did you mean to specify any of those?`?

Commit message: Add comments to tests

Signed-off-by: Ahdra Merali <ahdra.merali@quantumblack.com>

Patch to review:
diff --git a/RELEASE.md b/RELEASE.md
index 6ae6981e90..20bb0f06e3 100644
--- a/RELEASE.md
+++ b/RELEASE.md
@@ -15,6 +15,7 @@
 
 ## Bug fixes and other changes
 * Use default `False` value for rich logging `set_locals`, to make sure credentials and other sensitive data isn't shown in logs.
+* When using `kedro run -n [some_node]`, if `some_node` is missing a namespace the resulting error message will suggest the correct node name.
 * The Kedro IPython extension now surfaces errors when it cannot load a Kedro project.
 
 ## Upcoming deprecations for Kedro 0.19.0
diff --git a/kedro/pipeline/pipeline.py b/kedro/pipeline/pipeline.py
index 029f7aa428..f563de623a 100644
--- a/kedro/pipeline/pipeline.py
+++ b/kedro/pipeline/pipeline.py
@@ -5,6 +5,7 @@
 """
 import copy
 import json
+import re
 from collections import Counter, defaultdict
 from itertools import chain
 from typing import Dict, Iterable, List, Set, Tuple, Union
@@ -380,6 +381,21 @@ def only_nodes(self, *node_names: str) -> "Pipeline":
         """
         unregistered_nodes = set(node_names) - set(self._nodes_by_name.keys())
         if unregistered_nodes:
+            # check if unregistered nodes are available under namespace
+            namespaces = []
+            for name in node_names:
+                namespaces.extend(
+                    [
+                        node_name
+                        for node_name in self._nodes_by_name.keys()
+                        if re.match(rf"^.*\.{name}$", node_name)
+                    ]
+                )
+            if namespaces:
+                raise ValueError(
+                    f"Pipeline does not contain nodes named {list(unregistered_nodes)}. "
+                    f"Did you mean: {namespaces}?"
+                )
             raise ValueError(
                 f"Pipeline does not contain nodes named {list(unregistered_nodes)}."
             )
diff --git a/tests/pipeline/test_pipeline.py b/tests/pipeline/test_pipeline.py
index 14a01a20a0..5bc402c1a2 100644
--- a/tests/pipeline/test_pipeline.py
+++ b/tests/pipeline/test_pipeline.py
@@ -791,6 +791,56 @@ def test_only_nodes_unknown(self, pipeline_list_with_lists, target_node_names):
         with pytest.raises(ValueError, match=pattern):
             full.only_nodes(*target_node_names)
 
+    @pytest.mark.parametrize(
+        "namespaced_node", ["node1", "node2", "node3", "node4", "node5", "node6"]
+    )
+    def test_only_nodes_with_namespacing(
+        self, pipeline_with_namespaces, namespaced_node
+    ):
+        # Tests that error message will supply correct namespaces.
+        # Example of expected error:
+        # Pipeline does not contain nodes named ['node1']. Did you mean: ['katie.node1']?
+        pattern = (
+            rf"Pipeline does not contain nodes named \['{namespaced_node}'\]\. "
+            rf"Did you mean: \['.*\.{namespaced_node}'\]\?"
+        )
+        full = pipeline_with_namespaces
+        with pytest.raises(ValueError, match=pattern):
+            full.only_nodes(namespaced_node)
+
+    @pytest.mark.parametrize(
+        "namespaced_nodes", [("node1", "node2"), ("node3", "node4"), ("node5", "node6")]
+    )
+    def test_only_nodes_with_namespacing_multiple_args(
+        self, pipeline_with_namespaces, namespaced_nodes
+    ):
+        # Tests that error message will contain suggestions for all provided arguments.
+        # Example of expected error message:
+        # "Pipeline does not contain nodes named ['node1', 'node2'].
+        # Did you mean: ['katie.node1', 'lisa.node2']?"
+        pattern = rf"('.*\.{namespaced_nodes[0]}')+.*('.*\.{namespaced_nodes[1]}')+"
+        full = pipeline_with_namespaces
+        with pytest.raises(ValueError, match=pattern):
+            full.only_nodes(*namespaced_nodes)
+
+    @pytest.mark.parametrize(
+        "namespaced_nodes", [("node1", "invalid_node"), ("invalid_node", "node2")]
+    )
+    def test_only_nodes_with_namespacing_and_invalid_args(
+        self, pipeline_with_namespaces, namespaced_nodes
+    ):
+        # Tests error message will still contain namespace suggestions for correct arguments.
+        # regex is not specific to node names due to unspecified order
+        # Example of expected error message:
+        # "Pipeline does not contain nodes named ['node1', 'invalid_node'].
+        # Did you mean: ['katie.node1']?"
+        pattern = (
+            r"Pipeline does not contain nodes named \[.*\]\. Did you mean: \[.*\]\?"
+        )
+        full = pipeline_with_namespaces
+        with pytest.raises(ValueError, match=pattern):
+            full.only_nodes(*namespaced_nodes)
+
     def test_from_inputs(self, complex_pipeline):
         """F and H are inputs of node1, node2 and node3."""
         new_pipeline = complex_pipeline.from_inputs("F", "H")


Code review comments:
diff_hunk: @@ -791,6 +791,56 @@ def test_only_nodes_unknown(self, pipeline_list_with_lists, target_node_names):
         with pytest.raises(ValueError, match=pattern):
             full.only_nodes(*target_node_names)
 
+    @pytest.mark.parametrize(
+        "namespaced_node", ["node1", "node2", "node3", "node4", "node5", "node6"]
path: tests/pipeline/test_pipeline.py
text: @user1:
I think this would be better named as ` non_namespaced_node` instead of `namespaced_node`. As the test is trying to pass incorrect node names into a namespaced pipeline and catch the error.

@author:
Makes sense!

diff_hunk: @@ -380,6 +381,21 @@ def only_nodes(self, *node_names: str) -> "Pipeline":
         """
         unregistered_nodes = set(node_names) - set(self._nodes_by_name.keys())
         if unregistered_nodes:
+            # check if unregistered nodes are available under namespace
+            namespaces = []
+            for name in node_names:
path: kedro/pipeline/pipeline.py
text: @user1:
```suggestion
            for unregistered_node in unregistered_nodes:
```
Surely we only need to check through `unregistered_nodes`?

I know you already renamed the variables but I don't think `name` (which is the name of a node) vs. `node_name` (which is also the name of a node) is very clear 😅 I'd suggest something like `unregistered_node_name` or something to be a bit more explicit.

@author:
> 1. Elsewhere in kedro we use `difflib`, e.g. to [suggest catalog entry names](https://github.com/kedro-org/kedro/blob/main/kedro/io/data_catalog.py#L292) or CLI commands that are mistyped. Did you consider using that here rather than coding our own very simple matching algorithm? I don't know what the capabilities of `difflib` are so maybe it doesn't have anything suitable for this case, but worth checking.

`difflib` does offer some interesting capabilities, especially if we go for the route suggested in the second point, but I don't think the nature of its implementation works in this context. A quick test reveals the `get_close_matches` will omit the "correct" suggestions in favour for unrelated nodes that it computes as being more similar to the inputs 😬. 



@author:
> Surely we only need to check through unregistered_nodes?

Thanks for catching that! As a consequence the error message will no longer display the incorrect nodes in the order they were provided by the user - is this behaviour undesirable or should it be ignored?

@user1:
I think it's fine - the order isn't important here.

diff_hunk: @@ -380,6 +381,21 @@ def only_nodes(self, *node_names: str) -> "Pipeline":
         """
         unregistered_nodes = set(node_names) - set(self._nodes_by_name.keys())
         if unregistered_nodes:
+            # check if unregistered nodes are available under namespace
+            namespaces = []
+            for name in node_names:
+                namespaces.extend(
+                    [
+                        node_name
+                        for node_name in self._nodes_by_name.keys()
+                        if re.match(rf"^.*\.{name}$", node_name)
path: kedro/pipeline/pipeline.py
text: @user1:
A regex feels a bit heavy just for this - maybe it would be simpler to do just `node_name.endswith(f".{name}")`?

diff_hunk: @@ -791,6 +791,56 @@ def test_only_nodes_unknown(self, pipeline_list_with_lists, target_node_names):
         with pytest.raises(ValueError, match=pattern):
             full.only_nodes(*target_node_names)
 
+    @pytest.mark.parametrize(
+        "namespaced_node", ["node1", "node2", "node3", "node4", "node5", "node6"]
+    )
+    def test_only_nodes_with_namespacing(
+        self, pipeline_with_namespaces, namespaced_node
+    ):
+        # Tests that error message will supply correct namespaces.
+        # Example of expected error:
+        # Pipeline does not contain nodes named ['node1']. Did you mean: ['katie.node1']?
+        pattern = (
+            rf"Pipeline does not contain nodes named \['{namespaced_node}'\]\. "
+            rf"Did you mean: \['.*\.{namespaced_node}'\]\?"
+        )
+        full = pipeline_with_namespaces
path: tests/pipeline/test_pipeline.py
text: @user1:
I'd prefer just to use the `pipeline_with_namespaces`.