Make _preprocess_data stricter regarding label_namer.

anntzer · anntzer · commit b07377fdd4d8 · 2018-11-04T14:41:04.000+01:00
Assert that label_namer (if given) is a parameter in the function
signature (not "possibly" present via `**kwargs`).  This seems more
consistent with the expectations of the check later; in fact we can
now just remove that check.
diff --git a/lib/matplotlib/__init__.py b/lib/matplotlib/__init__.py
@@ -1589,9 +1589,17 @@ def func(ax, *args, **kwargs): ...
         The list of parameter names for which lookup into *data* should be
         attempted. If None, replacement is attempted for all arguments.
     label_namer : string, optional, default: None
-        If set e.g. to "namer", if a ``namer`` kwarg is passed as a string, and
-        a ``label`` kwarg is not passed, then pass the value of the ``namer``
-        kwarg as the ``label`` kwarg as well.
+        If set e.g. to "namer" (which must be a kwarg in the function's
+        signature -- not as ``**kwargs``), if the *namer* argument passed in is
+        a (string) key of *data* and no *label* kwarg is passed, then use the
+        (string) value of the *namer* as *label*. ::
+
+            @_preprocess_data(label_namer="foo")
+            def func(foo, label=None): ...
+
+            func("key", data={"key": value})
+            # is equivalent to
+            func.__wrapped__(value, label="key")
     """
 
     if func is None:  # Return the actual decorator.
@@ -1625,7 +1633,7 @@ def func(ax, *args, **kwargs): ...
     assert (replace_names or set()) <= set(arg_names) or varkwargs_name, (
         "Matplotlib internal error: invalid replace_names ({!r}) for {!r}"
         .format(replace_names, func.__name__))
-    assert label_namer is None or label_namer in arg_names or varkwargs_name, (
+    assert label_namer is None or label_namer in arg_names, (
         "Matplotlib internal error: invalid label_namer ({!r}) for {!r}"
             .format(label_namer, func.__name__))
 
@@ -1656,26 +1664,19 @@ def inner(ax, *args, data=None, **kwargs):
         bound.apply_defaults()
         del bound.arguments["data"]
 
-        all_kwargs = {**bound.arguments, **bound.kwargs}
         if needs_label:
-            if label_namer not in all_kwargs:
-                cbook._warn_external(
-                    "Tried to set a label via parameter {!r} in func {!r} but "
-                    "couldn't find such an argument.\n(This is a programming "
-                    "error, please report to the Matplotlib list!)".format(
-                        label_namer, func.__name__),
-                    RuntimeWarning)
+            all_kwargs = {**bound.arguments, **bound.kwargs}
+            # label_namer will be in all_kwargs as we asserted above that
+            # `label_namer is None or label_namer in arg_names`.
+            label = _label_from_arg(all_kwargs[label_namer], auto_label)
+            if "label" in arg_names:
+                bound.arguments["label"] = label
+                try:
+                    bound.arguments.move_to_end(varkwargs_name)
+                except KeyError:
+                    pass
             else:
-                label = _label_from_arg(all_kwargs[label_namer], auto_label)
-                if "label" in arg_names:
-                    bound.arguments["label"] = label
-                    try:
-                        bound.arguments.move_to_end(varkwargs_name)
-                    except KeyError:
-                        pass
-                else:
-                    bound.arguments.setdefault(
-                        varkwargs_name, {})["label"] = label
+                bound.arguments.setdefault(varkwargs_name, {})["label"] = label
 
         return func(*bound.args, **bound.kwargs)
 
diff --git a/lib/matplotlib/tests/test_preprocess_data.py b/lib/matplotlib/tests/test_preprocess_data.py
@@ -58,26 +58,6 @@ def func_no_ax_args(*args, **kwargs): pass
     with pytest.raises(AssertionError):
         _preprocess_data(label_namer="z")(func_args)
 
-    # but "ok-ish", if func has kwargs -> will show up at runtime :-(
-    _preprocess_data(label_namer="z")(func_kwargs)
-    _preprocess_data(label_namer="z")(func_no_ax_args)
-
-
-def test_label_namer_only_if_data():
-    """label_namer should only apply when data is passed."""
-
-    def real_func(x, y):
-        pass
-
-    @_preprocess_data(label_namer="x")
-    def func(*args, **kwargs):
-        real_func(**kwargs)
-
-    func(None, x="a", y="b")
-    with pytest.raises(TypeError):
-        # This sets a label although the function can't handle it.
-        func(None, x="a", y="b", data={"a": "A", "b": "B"})
-
 
 @pytest.mark.parametrize('func', all_funcs, ids=all_func_ids)
 def test_function_call_without_data(func):
@@ -178,42 +158,6 @@ def func_replace_all(ax, x, y, ls="x", label=None, w="NOT"):
         func_replace_all(None, x="a", y="b", w="x", label="text", data=data) ==
         "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: text")
 
-    @_preprocess_data(label_namer="y")
-    def func_varags_replace_all(ax, *args, **kwargs):
-        all_args = [None, None, "x", None, "xyz"]
-        for i, v in enumerate(args):
-            all_args[i] = v
-        for i, k in enumerate(["x", "y", "ls", "label", "w"]):
-            if k in kwargs:
-                all_args[i] = kwargs[k]
-        x, y, ls, label, w = all_args
-        return "x: %s, y: %s, ls: %s, w: %s, label: %s" % (
-            list(x), list(y), ls, w, label)
-
-    # in the first case, we can't get a "y" argument,
-    # as we don't know the names of the *args
-    assert (func_varags_replace_all(None, x="a", y="b", w="x", data=data) ==
-            "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: b")
-    assert (
-        func_varags_replace_all(None, "a", "b", w="x", label="", data=data) ==
-        "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: ")
-    assert (
-        func_varags_replace_all(None, "a", "b", w="x", label="text",
-                                data=data) ==
-        "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: text")
-    assert (
-        func_varags_replace_all(None, x="a", y="b", w="x", label="",
-                                data=data) ==
-        "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: ")
-    assert (
-        func_varags_replace_all(None, x="a", y="b", w="x", label="text",
-                                data=data) ==
-        "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: text")
-
-    with pytest.warns(RuntimeWarning):
-        assert (func_varags_replace_all(None, "a", "b", w="x", data=data) ==
-                "x: [1, 2], y: [8, 9], ls: x, w: xyz, label: None")
-
 
 def test_no_label_replacements():
     """Test with "label_namer=None" -> no label replacement at all"""
