Feat/allow more flexible arguments #1035
Conversation
|
@lotruheawea If you want to take a look, let me know! I'll leave this open for a few days and merge it next week and do a release. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1035 +/- ##
====================================
Coverage 77% 78%
====================================
Files 31 31
Lines 3667 3680 +13
====================================
+ Hits 2858 2887 +29
+ Misses 809 793 -16
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This generally looks very neat to me. I really like the usage of the inspect library in apply_args. I have only minor comments concerning types and the test setup.
Also I am wondering if it would be helpful to add an example for the verifier using a partial state_handler using a fixture (as I posted in the issue). This could highlight a usecase for potential users.
Instead of doing basic checks on the signature of functional argument, create a utility which will be responsible for passing arguments through to the functional argument. Signed-off-by: JP-Ellis <[email protected]>
The first support for functional arguments was very strict and did not
allow mixtures of positional and keyword arguments, and also did not
allow functions to have optional arguments.
This commit introduces a rewrite of the way arguments are applied. The
benefit is complete flexibility, with position, keyword and variadic
arguments all being supported.
In order to support this though, function _must_ have argument names
that form a subset of the `MessageProducerArgs` and `StateHandlerArgs`
typed dictionaries.
BREAKING CHANGE: The signature of functional arguments must form a
subset of the `MessageProducerArgs` and `StateHandlerArgs` typed
dictionaries.
Signed-off-by: JP-Ellis <[email protected]>
To ensure the blog post remains relevant, update the blog post to reflect the most recent changes. Add a notice at the bottom indicating that there was an update. Signed-off-by: JP-Ellis <[email protected]>
Thanks to @lotruheawea for spotting some missed renames. Co-authored-by: lotruheawea <[email protected]>
Instead of combining things into strings or lists, use a consistent `NamedTuple` to capture the mixture of arguments. Signed-off-by: JP-Ellis <[email protected]>
Signed-off-by: JP-Ellis <[email protected]>
Signed-off-by: JP-Ellis <[email protected]>
JP-Ellis
force-pushed
the
feat/allow-more-flexible-arguments
branch
from
aa3944a to
371e344
Compare
📝 Summary
Changes the handling of functional arguments to be much more flexible, allowing for default arguments.
🚨 Breaking Changes
Previously, the functional arguments used strictly position arguments and did not care about the name of the arguments. It also performed checks on the number of parameters (irrespective of the presence of default arguments).
The new behaviour allows for default arguments and variadic arguments (
*args,**kwargs), and is a lot more lenient as to the order of arguments. HOWEVER, the arguments are matched by name as defined in theMessageProducerArgsandStateHandlerArgstyped dictionaries.Specifically, this means that if you previously used
paramsas a positional argument, you must rename it toparameters.🔥 Motivation
To allow more flexible support of functional arguments, including allowing default arguments, and relax ordering requirements.
🔨 Test Plan
Tests have been updated to make use of the new feature.
🔗 Related issues/PRs