Repository: LLNL/hatchet

Issue:
improve error messages in tree()
For example, if a specified `metric_column` doesn't exist in the dataframe, raise `KeyError()` with a more user-friendly error. Possibly add other error messages for deprecated parameters as well.

Discussion:

Patch:
diff --git a/hatchet/external/console.py b/hatchet/external/console.py
index 4be2194..4d89370 100644
--- a/hatchet/external/console.py
+++ b/hatchet/external/console.py
@@ -59,6 +59,13 @@ class ConsoleRenderer:
         self.highlight = kwargs["highlight_name"]
         self.invert_colormap = kwargs["invert_colormap"]
 
+        if self.metric not in dataframe.columns:
+            raise KeyError(
+                "metric_column={} does not exist in the dataframe, please select a valid column.".format(
+                    self.metric
+                )
+            )
+
         if self.invert_colormap:
             self.colors.colormap.reverse()
 
diff --git a/hatchet/graphframe.py b/hatchet/graphframe.py
index ddaa8b9..0c68196 100644
--- a/hatchet/graphframe.py
+++ b/hatchet/graphframe.py
@@ -496,9 +496,6 @@ class GraphFrame:
         expand_names="expand_name",
         context="context_column",
         invert_colors="invert_colormap",
-        color="",
-        threshold="",
-        unicode="",
     )
     def tree(
         self,
@@ -512,9 +509,6 @@ class GraphFrame:
         depth=10000,
         highlight_name=False,
         invert_colormap=False,
-        color=None,  # remove in next release
-        threshold=None,  # remove in next release
-        unicode=None,  # remove in next release
     ):
         """Format this graphframe as a tree and return the resulting string."""
         color = sys.stdout.isatty()
diff --git a/hatchet/util/deprecated.py b/hatchet/util/deprecated.py
index 134fb95..4ab8ad4 100644
--- a/hatchet/util/deprecated.py
+++ b/hatchet/util/deprecated.py
@@ -4,7 +4,6 @@
 # SPDX-License-Identifier: MIT
 
 import functools
-import warnings
 
 
 def deprecated_params(**old_to_new):
@@ -29,16 +28,16 @@ def rename_kwargs(fname, old_to_new, kwargs):
 
             # if parameter has been removed
             if not new:
-                warnings.warn(
-                    '{}() parameter "{}=" has been removed.'.format(fname, old),
-                    FutureWarning,
+                raise ValueError(
+                    '{}() parameter "{}=" will be removed in a future release. Please remove it from this script.'.format(
+                        fname, old
+                    )
                 )
             # if parameter has been deprecated and renamed
             else:
-                warnings.warn(
+                raise ValueError(
                     '{}() parameter "{}=" has been deprecated, please use "{}=".'.format(
                         fname, old, new
-                    ),
-                    FutureWarning,
+                    )
                 )
                 kwargs[new] = kwargs.pop(old)


Test patch:
diff --git a/hatchet/tests/graphframe.py b/hatchet/tests/graphframe.py
index 0be1a25..5f8572f 100644
--- a/hatchet/tests/graphframe.py
+++ b/hatchet/tests/graphframe.py
@@ -909,18 +909,12 @@ def test_depth(mock_graph_literal):
 def test_tree_deprecated_parameters(mock_graph_literal):
     gf = GraphFrame.from_literal(mock_graph_literal)
 
-    with pytest.warns(FutureWarning):
-        gf.tree(color=True, metric="time")
-
-    with pytest.warns(FutureWarning):
+    with pytest.raises(ValueError):
         gf.tree(invert_colors=True)
 
-    with pytest.warns(FutureWarning):
+    with pytest.raises(ValueError):
         gf.tree(name="name")
 
-    with pytest.warns(FutureWarning):
-        gf.tree(threshold=0.2)
-
     with pytest.raises(TypeError):
         gf.tree(metric="time", metric_column="time")