New exception "ValueError: unused keyword argument" breaks use case

Create issue
Issue #357 resolved
Makyen created an issue

Unfortunately, the change to unconditionally raising this ValueError: unused keyword argument exception breaks a significant number of our uses of this package. As a temporary fix to keep us operational, I've had to pin the version we use to 2019.12.20. Fortunately, this was detected in our CI testing.

Our project is SmokeDetector, a bot that scans new and edited posts on Stack Exchange/Stack Overflow for spam and rude/abusive content.

We have over 22,000 regular expressions. The vast majority of those are stored in files external to the Python code. People have the ability to give commands to the bot to dynamically add or remove regular expressions while the bot is running. The expressions are permitted to use named lists, currently only a list of cities, which we make available to the expressions when we .compile() them. The expressions are not segregated out between ones which use the cities list and those that don't. We just pass the list to .compile() and the regular expression can use the list, or not. Thus, for a large number of our calls to .compile(), the regular expression doesn’t use the list and the keyword argument is unused.

Obviously, that means that unconditionally raising the ValueError: unused keyword argument exception results in the bot crashing upon startup when it attempts to compile the regular expressions.

While I can understand the desire to have this exception raised as a method of protecting against a small class of potential typos (issue #354 indicated problems remembering to use flag or flags), it would be better if we could also accommodate the use case where it's known and intended that keyword arguments containing named lists may not be used by the regex which is currently being compiled. Perhaps with an optional argument to disable raising the exception.

Comments (3)

  1. Mark Amery

    Dang. I was aware of the compatibility break as a theoretical possibility when I proposed this change but hoped that nobody was using this feature in the way that you are and that I’d get away with it. Apparently, I was wrong!

    If the API were being created anew, I think the right answer to this problem would be to have the named lists be passed as a dictionary to a named_lists parameter, not as keyword arguments. But now we have backwards compatibility to consider, so it’s more complicated.

    I’m not sure what the right solution is. We could:

    • introduce that argument now, in parallel to the ability to pass kwargs (and have unneeded lists be ignored iff they’re passed via that argument), or

    • introduce it as a replacement for passing kwargs, requiring all programs that used the old API to change how they pass keyword lists but maintaining a consistent API, or

    • introduce a flag as you suggest, or

    • revert the change entirely and live with the lack of typo protection

    Option 2, IMO, results in the cleanest API, also makes the compatibility break more extensive. Option 4, meanwhile, is the only one that avoids breaking compatibility entirely.

  2. Log in to comment