Fix CapturedCallable to work better with kwargs

[antonymilne]

Description

#114 uncovered a bug with CapturedCallable. In short, this would work ok:

function = lambda **kwargs: kwargs["a"]

CapturedCallable(function)(a=2) # a=2 only provided at runtime

And this would not:

CapturedCallable(function, a=2)() # a=2 is provided upfront

Now they should both work ok.

We have already had a couple of very small bugs that I've known about for a while, where the following sorts of functions do not work with CapturedCallable:

1. a function with variadic positional args like def function(*args)
2. a function with positional-only arguments like def function(a, /)

Fixing the new **kwargs bug, which is an important case, makes it much harder to fix these much less important cases, and so I've decided to just not support them at all. There's now an explicit check to make sure no one tries to use CapturedCallable with those sorts of functions (very unlikely they'd want to do so anyway).

Tests have been updated to be much more thorough so that we now test every case on all 3 sorts of valid function positional_or_keyword_function, keyword_only_function, var_keyword_function.

Thanks to @maxschulz-COL for spotting the bug and for his initial fix which is the basis for this!

Checklist

• I have not referenced individuals, products or companies in any commits, directly or indirectly
• I have not added data or restricted code in any commits, directly or indirectly
• I have updated the docstring of any public function/class/model changed
• I have added the PR number to the change description in the changelog fragment, e.g. Enable feature XXX ([#1](https://github.com/mckinsey/vizro/pull/1)) (if applicable)
• I have added tests to cover my changes (if applicable)

Types of changes

• Docs/refactoring (non-breaking change which improves codebase)
• Bug fix (non-breaking change which fixes an issue)
• New feature (non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)

Notice

• I acknowledge and agree that, by checking this box and clicking "Submit Pull Request":

  • I submit this contribution under the Apache 2.0 license and represent that I am entitled to do so on behalf of myself, my employer, or relevant third parties, as applicable.
  • I certify that (a) this contribution is my original creation and / or (b) to the extent it is not my original creation, I am authorized to submit this contribution on behalf of the original creator(s) or their licensees.
  • I certify that the use of this contribution as authorized by the Apache 2.0 license does not violate the intellectual property rights of anyone else.

[petar-qb]

I like the compromise this PR brings (fixing a bug with variadic keyword arguments while not supporting positional only and variadic positional arguments) and think this will not adversely affect custom actions and custom graphs since we always send function arguments through the Parameter, and custom action mechanism in a keyword format.

Also thanks for the comments and tests in the code 💯 (they help lot to understand the problem and implemented solution).

So, +1 approval from my side. 🥇

Just update the changelog.d directory (with hatch run docs:changelod) with described bugfix (cuz it affects our users) and tick corresponding checkboxes under the Checklist and Types of changes in the PR Description 😄

[maxschulz-COL]

Really great work! I just have one concern which I may have agreed to last night not grasping the full consequence, but maybe I am wrong. Let's see!

Thanks also for the extensive tests, really a joy to read actually!

[huong-li-nguyen]

Thanks for this, looks great! 🚀 Was mostly reading through this out of interest, the tests really helped understanding it. Is it worth adding a note to the docs for the custom charts/actions or do you think the error messages suffice to provide guidance? Either way is fine for me 👍

[antonymilne]

Thanks for this, looks great! 🚀 Was mostly reading through this out of interest, the tests really helped understanding it. Is it worth adding a note to the docs for the custom charts/actions or do you think the error messages suffice to provide guidance? Either way is fine for me 👍

I think no need for anything in the narrative docs, since a user is so unlikely to ever try to do CapturedCallable on a function where it doesn't work that having any note on this is more likely to just confuse things. I will make a note of it in the docstring though!