Repository: zincware/ZnTrack

Issue:
`UserWarning: Node name should not contain '_'.` occurs when using groups

The warning should not come up when defining a group with no other `_` in use.

Discussion:

Solution patch:
diff --git a/zntrack/node.py b/zntrack/node.py
index 889a274b..5082659b 100644
--- a/zntrack/node.py
+++ b/zntrack/node.py
@@ -194,7 +194,11 @@ def from_rev(
 
         lazy_values["name"] = name
         lazy_values["always_changed"] = None  # TODO: read the state from dvc.yaml
-        instance = cls(**lazy_values)
+        with warnings.catch_warnings():
+            warnings.filterwarnings(
+                "ignore", message=".*should not contain '_'*", category=UserWarning
+            )
+            instance = cls(**lazy_values)
         if remote is not None or rev is not None:
             import dvc.api
 
diff --git a/zntrack/project.py b/zntrack/project.py
index 7e927fc7..7024d70c 100644
--- a/zntrack/project.py
+++ b/zntrack/project.py
@@ -250,6 +250,12 @@ def group(self, *names: str):
             while (name,) in self.groups:
                 name = f"Group{int(name[5:]) + 1}"
             names = (name,)
+        else:
+            if any("_" in name for name in names):
+                warnings.warn(
+                    "Group name should not contain '_'. "
+                    f"Consider using '-' instead for {','.join(names)}."
+                )
 
         with super().group(*names) as znflow_grp:
             group = Group(names=znflow_grp.names)


Test patch:
diff --git a/tests/unit_tests/test_node_name.py b/tests/unit_tests/test_node_name.py
index 97c8d0b4..38bcf050 100644
--- a/tests/unit_tests/test_node_name.py
+++ b/tests/unit_tests/test_node_name.py
@@ -1,3 +1,5 @@
+import warnings
+
 import pytest
 from dvc.stage.exceptions import InvalidStageName
 
@@ -150,12 +152,103 @@ def test_forbidden_node_names(proj_path, char):
     with zntrack.Project():
         # https://github.com/iterative/dvc/blob/main/tests/func/test_run.py#L372-L375C32
         with pytest.raises(InvalidStageName):
-            MyNode(name=f"copy_name-{char}")
+            MyNode(name=f"copy-name-{char}")
 
 
 @pytest.mark.parametrize("char", ["@:", "#", "$", ":", "/", "\\", ".", ";", ","])
 def test_forbidden_group_names(proj_path, char):
     project = zntrack.Project()
     with pytest.raises(InvalidStageName):
-        with project.group(f"copy_name-{char}"):
+        with project.group(f"copy-name-{char}"):
+            MyNode()
+
+
+# --- NODE NAME TESTS --- #
+
+
+@pytest.mark.parametrize("nodename", ["A_B_C"])
+def test_node_name_warns_on_underscore(proj_path, nodename):
+    with zntrack.Project():
+        with pytest.warns(Warning, match="Node name should not contain '_'"):
+            MyNode(name=nodename)
+
+
+@pytest.mark.parametrize("nodename", ["ABC"])
+def test_node_name_no_warning(proj_path, nodename):
+    project = zntrack.Project()
+
+    with project:
+        with warnings.catch_warnings():
+            warnings.simplefilter("error")
+            node = MyNode(name=nodename)
+
+    project.build()
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        zntrack.from_rev(name=node.name)
+
+
+# --- GROUP NAME TESTS --- #
+
+
+@pytest.mark.parametrize("groupname", ["A_B_C"])
+def test_group_name_warns_on_underscore(proj_path, groupname):
+    project = zntrack.Project()
+
+    with pytest.warns(Warning, match="Group name should not contain '_'"):
+        with project.group(groupname):
             MyNode()
+
+
+@pytest.mark.parametrize("groupname", ["ABC"])
+def test_group_name_no_warning(proj_path, groupname):
+    project = zntrack.Project()
+
+    with project:
+        with warnings.catch_warnings():
+            warnings.simplefilter("error")
+            with project.group(groupname):
+                node = MyNode()
+
+    project.build()
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        zntrack.from_rev(name=node.name)
+
+
+# --- COMBINED GROUP+NODE NAME TESTS --- #
+
+
+@pytest.mark.parametrize(
+    "groupname, nodename",
+    [
+        ("A_B_C", "A_B_C"),
+        ("A", "A_B_C"),
+        ("A_B_C", "A"),
+    ],
+)
+def test_group_and_node_name_warns_on_underscore(proj_path, groupname, nodename):
+    project = zntrack.Project()
+
+    with pytest.warns(Warning, match="should not contain '_'"):
+        with project.group(groupname):
+            MyNode(name=nodename)
+
+
+@pytest.mark.parametrize("groupname, nodename", [("ABC", "DEF")])
+def test_group_and_node_name_no_warning(proj_path, groupname, nodename):
+    project = zntrack.Project()
+
+    with project:
+        with warnings.catch_warnings():
+            warnings.simplefilter("error")
+            with project.group(groupname):
+                node = MyNode(name=nodename)
+
+    project.build()
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        zntrack.from_rev(name=node.name)