fix: modify error message and add warning (#250)

* fix: add warning for disallowed names

* fix: modify error message for disallowed names

* fix: mention disallowed name in warning message

Co-authored-by: Andrzej Novak <[email protected]>

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fix: modify warning to display disallowed name

* fix: modify warning for efficiency

* test: add test for disallowed names

* test: modify tests for disallowed names

* fix: store disallowed names in set

Co-authored-by: Henry Schreiner <[email protected]>

Co-authored-by: Andrzej Novak <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Henry Schreiner <[email protected]>

src/hist/basehist.py

@@ -94,6 +94,13 @@ def __init__(
                 warnings.warn(msg)
                 storage = storage()
             super().__init__(*args, storage=storage, metadata=metadata)  # type: ignore
+
+            disallowed_names = {"weight", "sample", "threads"}
+            for ax in self.axes:
+                if ax.name in disallowed_names:
+                    disallowed_warning = f"{ax.name} is a protected keyword and cannot be used as axis name"
+                    warnings.warn(disallowed_warning)
+
             valid_names = [ax.name for ax in self.axes if ax.name]
             if len(valid_names) != len(set(valid_names)):
                 raise KeyError(
@@ -203,7 +210,9 @@ def fill(
         }
 
         if set(data_dict) != set(range(len(args), self.ndim)):
-            raise TypeError("All axes must be accounted for in fill")
+            raise TypeError(
+                "All axes must be accounted for in fill, you may have used a disallowed name in the axes"
+            )
 
         data = (data_dict[i] for i in range(len(args), self.ndim))
 

tests/test_axis.py

@@ -1,4 +1,6 @@
-from hist import axis
+import pytest
+
+from hist import axis, hist
 
 
 def test_axis_names():
@@ -51,3 +53,13 @@ def test_axis_flow():
     assert axis.Integer(0, 8, flow=False, overflow=True) == axis.Integer(
         0, 8, underflow=False
     )
+
+
+def test_axis_disallowed_names():
+
+    with pytest.warns(UserWarning):
+        hist.Hist(axis.Regular(10, 0, 10, name="weight"))
+    with pytest.warns(UserWarning):
+        hist.Hist(axis.Regular(10, 0, 10, name="sample"))
+    with pytest.warns(UserWarning):
+        hist.Hist(axis.Regular(10, 0, 10, name="threads"))