Add support to pass args/kwargs to callables from Accessor #940
Conversation
… Accessor jieter#939. Additionally added the required unit test
jieter
changed the title
django_tables2/utils.py
Outdated
| self.callable_kwargs = callable_kwargs | ||
| super().__init__() | ||
|
|
||
| def __new__(cls, value, *args, **kwargs): |
There was a problem hiding this comment.
__new__ did not have *args, **kwargs before, does the fix need that?
There was a problem hiding this comment.
__new__ and __init__ need to have compatible signatures, I updated it so that it is explicit now
| @@ -394,7 +399,7 @@ def resolve(self, context, safe=True, quiet=False): | |||
| if safe and getattr(current, "alters_data", False): | |||
| raise ValueError(self.ALTERS_DATA_ERROR_FMT.format(method=current.__name__)) | |||
| if not getattr(current, "do_not_call_in_templates", False): | |||
| current = current() | |||
| current = current(*self.callable_args, **self.callable_kwargs) | |||
There was a problem hiding this comment.
I'm not sure on the callable_-prefix, can we simplify this by just calling them self.args and self.kwargs?
There was a problem hiding this comment.
I kind of like it being more verbose as it tells the user what those args and kwargs are being used for. If you disagree, I can make them be args and kwargs no problem. Another option is to not wild-card them and have the user pass them as a list/dict instead, this better indicates that they are "pass-through" args/kwargs. I am going to change it to the later and you let me know what you think
| callable_kwargs = {"kwarg1": "val1", "kwarg2": "val2"} | ||
| obj = MyClass() | ||
| result = Accessor("method", *callable_args, **callable_kwargs).resolve(obj) | ||
| self.assertEqual(result, (callable_args, callable_kwargs)) |
There was a problem hiding this comment.
Next step: supporting Accessor as arg/kwarg to support even more dynamic constructs 😬
|
Thanks for your contribution @JordanHyatt. Please have a look at my questions. |
…ssed to the constructor instead of a str object
Fixes: #939
Implemented the feature request and wrote the required unit text