diff --git a/README.md b/README.md index 55243fb..61e4263 100644 --- a/README.md +++ b/README.md @@ -17,6 +17,7 @@ from typing import Annotated, Union, Optional, Any def public_function( # validate against built-in or custom types a: str, + /, # support for positional-only arguments # support for type unions b: Union[int, float], # or from Python 3.10 `int | float` # validate type of container items @@ -61,7 +62,7 @@ def public_function( return {"a":a, "b":b, "c":c, "d":d, "e":e, "f":f, "g":g, "h":h, "i":i, "args":args, "j":j, "k":k} public_function( - # NB parameters 'a' through 'i' can be passed positionally + # NB 'a' must be passed positionally, 'b' through 'i' can be passed positionally "zero", # a 1.0, # b {"two": 2}, # c @@ -95,7 +96,7 @@ returns: And if there are invalid inputs... ```python public_function( - a=["not a string"], # INVALID + ["not a string"], # INVALID b="not an int or a float", # INVALID c={2: "two"}, # INVALID, key not a str and value not an int or float d=3.2, # valid input @@ -134,19 +135,22 @@ public_function( {"two": 2}, 3.2, # no argument passed for required positional args 'e', 'f', 'g', 'h' and 'i' - a="a again", # passing multiple values for parameter 'a' - # no argument passed for required keyword arg 'j' + a="a again", # 'a' is positional-only: cannot be passed as a kwarg unless sig has **kwargs + c={"three": 3}, # passing multiple values for 'c' not_a_kwarg="not a kwarg", # including an unexpected kwarg + # no argument passed for required keyword arg 'j' ) ``` raises: ``` InputsError: Inputs to 'public_function' do not conform with the function signature: -Got multiple values for argument: 'a'. +Got multiple values for argument: 'c'. Got unexpected keyword argument: 'not_a_kwarg'. +Got positional-only argument as keyword argument (and signature makes no provision for **kwargs that would otherwise receive it): 'a'. + Missing 5 positional arguments: 'e', 'f', 'g', 'h' and 'i'. Missing 1 keyword-only argument: 'j'. @@ -232,6 +236,18 @@ In short, if you only want to validate the type of function inputs then Pydantic * `collections.abc.Mapping` * packing and optionally coercing, parsing and validating packed objects, i.e. objects received to, for example, *args and **kwargs. +* full verification of signature compliance in accordance with standard Python, i.e. + verifies: + - no excess positional arguments + - no unexpected keyword arguments (if the signature does not provide for **kwargs) + - no missing 'required' arguments (i.e. arguments that do not otherwise have a default value) + - no duplicate arguments + - postional-only arguments are passed positionally. (If a keyword argument is passed + with the same name as a positional-only argument then it will be considered valid if + the signature provides for **kwargs (and in this case it will be received by + **kwargs), whilst if the signature does not provide for **kwargs then the argument + will be considered invalid.) + - positional arguments are passed either positionally or by keyword argument `valimp` does NOT support: - Validation of subscripted types in `collections.abc.Callable`. Any subscriptions to @@ -241,12 +257,6 @@ In short, if you only want to validate the type of function inputs then Pydantic verify that an object passed to a parameter annotated as `Callable` is in fact callable). -`valimp` does NOT currently support: - - Positional-only arguments. Any '/' in the signature (to define - positional-only arguments) will be ignored. Consequently valimp DOES - allow intended positional-only arguments to be passed as keyword - arguments. - The library has been built with development in mind and PRs are very much welcome! ## License diff --git a/docs/tutorials/tutorial.ipynb b/docs/tutorials/tutorial.ipynb index 428d8a2..85d823e 100644 --- a/docs/tutorials/tutorial.ipynb +++ b/docs/tutorials/tutorial.ipynb @@ -1714,6 +1714,7 @@ "* A keyword argument is passed that is not represented in the signature (unexpected keyword argument).\n", "* More arguments are passed positionally than accommodated for by the signature (excess positional arguments).\n", "* A parameter is passed both positionally and as a keyword argument (got multiple values).\n", + "* A positional-only argument is passed as a keyword argument. (Note that if a keyword argument is passed with the same name as a positional-only argument then it will be considered _valid_ if the signature provides for **kwargs (and in this case it will be received by those **kwargs), whilst if the signature does not provide for **kwargs then the error will be included in the raised `InputsError`.)\n", "\n", "All signature errors are advised in the error message, together with any errors relating to invalid types." ] @@ -1727,8 +1728,10 @@ "source": [ "@parse\n", "def pf(\n", - " a: int,\n", + " a: int, # positional_only\n", + " /,\n", " b: int,\n", + " c: int,\n", " *,\n", " kw_a: int,\n", "):\n", @@ -1742,7 +1745,7 @@ "metadata": {}, "outputs": [], "source": [ - "pf(3, \"not an int\", 5, a=3, not_a_kwarg=3)" + "pf(3, \"not an int\", 4, 5, a=3, c=2, not_a_kwarg=3)" ] }, { @@ -1754,17 +1757,19 @@ "---------------------------------------------------------------------------\n", "InputsError Traceback (most recent call last)\n", "Cell In[47], line 1\n", - "----> 1 pf(3, \"not an int\", 5, a=3, not_a_kwarg=3)\n", + "----> 1 pf(3, \"not an int\", 4, 5, a=3, c=2, not_a_kwarg=3)\n", "\n", "InputsError: Inputs to 'pf' do not conform with the function signature:\n", "\n", - "Got multiple values for argument: 'a'.\n", + "Got multiple values for argument: 'c'.\n", "\n", "Received 1 excess positional argument as:\n", "\t'5' of type .\n", "\n", "Got unexpected keyword argument: 'not_a_kwarg'.\n", "\n", + "Got positional-only argument as keyword argument (and signature makes no provision for **kwargs that would otherwise receive it): 'a'.\n", + "\n", "Missing 1 keyword-only argument: 'kw_a'.\n", "\n", "The following inputs to 'pf' do not conform with the corresponding type annotation:\n", @@ -1797,7 +1802,7 @@ "\n", "InputsError: Inputs to 'pf' do not conform with the function signature:\n", "\n", - "Missing 1 positional argument: 'b'.\n", + "Missing 2 positional arguments: 'b' and 'c'.\n", "```" ] }, diff --git a/src/valimp/valimp.py b/src/valimp/valimp.py index 743c48e..68d4155 100644 --- a/src/valimp/valimp.py +++ b/src/valimp/valimp.py @@ -10,12 +10,6 @@ The `parse_cls`decorator provides the same functionality for inputs to dataclasses. -Valimp does NOT currently support: - - Positional-only arguments. Any '/' in the signature (to define - positional-only arguments) will be ignored. Consequently valimp DOES - allow intended positional-only arguments to be passed as keyword - arguments. - See the tutorial for a walk-through of all functionality: https://github.com/maread99/valimp/blob/master/docs/tutorials/tutorial.ipynb @@ -770,13 +764,15 @@ def get_missing_arg_error(missing: list[str], *, positional: bool = True) -> Typ ) -def validate_against_signature( +def validate_against_signature( # noqa: C901 args_as_kwargs: dict[str, Any], kwargs: dict[str, Any], req_args: list[str], req_kwargs: list[str], excess_args: tuple[Any, ...], excess_kwargs: list[str], + posonly: list[str], + posonly_as_kwarg: list[str], ) -> list[TypeError]: """Validate inputs against arguments expected by signature. @@ -787,13 +783,14 @@ def validate_against_signature( name, value as received input (i.e. as if were received as a keyword argument). - NB module does not support positional-only arguments (i.e. these - could have been receieved as keyword args). - kwargs - Inputs for arguments receieved as keyword arguments. Key + Inputs for arguments received as keyword arguments. Key as argument name, value as received input. + NB should not include any keyword input matching the name of a + positional-only parameter (such inputs should be included to + `posonly_as_kwarg`). + req_args List of names of required positional arguments. @@ -808,6 +805,14 @@ def validate_against_signature( Names of any received kwargs that are not accommodated by the signature. + posonly + List of names of all positional-only parameters. + + posonly_as_kwarg + List of names of positional-only parameters that were invalidly + received as keyword arguments (i.e. as a keyword argument that + cannot be absorbed by a **kwargs parameter). + Returns ------- errors @@ -848,9 +853,35 @@ def validate_against_signature( ) ) + # positional-only arguments invalidly received as keyword arguments + if posonly_as_kwarg: + argument = "argument" if len(posonly_as_kwarg) == 1 else "arguments" + it_them = "it" if len(posonly_as_kwarg) == 1 else "them" + errors.append( + TypeError( + f"Got positional-only {argument} as keyword {argument} (and" + " signature makes no provision for **kwargs that would otherwise" + f" receive {it_them}): {args_name_inset(posonly_as_kwarg)}." + ) + ) + # missing required arguments all_as_kwargs = args_as_kwargs | kwargs - missing = [a for a in req_args if a not in all_as_kwargs] + missing = [] + for a in req_args: + if a in posonly_as_kwarg: + continue # already considered in `posonly_as_kwarg` TypeError + # a positional-only argument can only be satisfied by being received + # positionally - it is missing if it was not received positionally. + posonly_missing = a in posonly and a not in args_as_kwargs + if posonly_missing or a not in all_as_kwargs: + # Note that `a not in all_as_kwargs` is not enough as if a + # keyword argument was validly passed with the same name as a + # positional-only argument (i.e. if the sig included **kwargs) + # then the name of the missing positional-only arg *would* appear + # in `all_as_kwargs`. + missing.append(a) + if missing: errors.append(get_missing_arg_error(missing, positional=True)) @@ -1000,6 +1031,50 @@ def get_unreceived_kwargs( return {k: v for k, v in spec.kwonlydefaults.items() if k not in names_received} +def apply_coerce_and_parser( + name: str, + obj: Any, + hint: type[Any] | typing._Final, + params: dict[str, Any], +) -> Any: + """Apply any `Coerce` and `Parser` in metadata of a hint to an input. + + Parameters + ---------- + name + Name of the argument being parsed. + + obj + Input object to coerce and/or parse. + + hint + Type hint, possibly wrapped in `typing.Annotated`, against which + `obj` was validated. Any `Coerce` and `Parser` instances in the + metadata are applied in the order in which they are defined. + + params + Shallow copy of prior inputs that have already been parsed and, if + applicable, coerced. Passed through to any `Parser` function. + + Returns + ------- + Any + `obj` as coerced and/or parsed. Returned unchanged if `hint` is not + wrapped in `typing.Annotated`. + """ + if not is_annotated(hint): + return obj + for data in hint.__metadata__: + # let order of coercion and parsing depend on their order within metadata + if obj is not None and isinstance(data, Coerce): + obj = data.coerce_to(obj) + if isinstance(data, Parser): + if obj is None and not data.parse_none: + continue + obj = data.function(name, obj, params) + return obj + + def parse( # noqa: C901 f=None, *, @@ -1035,6 +1110,14 @@ def parse( # noqa: C901 ) name_extra_args = "_" + spec.varargs if spec.varargs is not None else "_a5f12_3adz" + # necessary to interrogate the signature as `inspect.getfullargspec` does not + # distinguish positional-only parameters (they are included to `spec.args`). + posonly_args = [ + name + for name, param in inspect.signature(f).parameters.items() + if param.kind is inspect.Parameter.POSITIONAL_ONLY + ] + @functools.wraps(f) def wrapped_f(*args, **kwargs) -> Any: # noqa: C901, PLR0912 hints_ = hints.copy() @@ -1042,10 +1125,10 @@ def wrapped_f(*args, **kwargs) -> Any: # noqa: C901, PLR0912 # handle extra args extra_args = args[len(spec.args) :] - if spec.varargs is None: # no provision for extra args, e.g. no *args in sig + if spec.varargs is None: # no variadic positional parameter (no *args in sig) excess_args = extra_args extra_args = () - else: # extra args provided for, e.g. with *args + else: # extra args provided for, e.g. within *args excess_args = () # add a hint for each extra arg hint = hints.get(spec.varargs, False) @@ -1057,34 +1140,91 @@ def wrapped_f(*args, **kwargs) -> Any: # noqa: C901, PLR0912 if hint: del hints_[spec.varargs] + # Remove any keyword input that matches a positional-only parameter name: + # it does not bind to that name, rather it will either be absorbed by a + # **kwargs parameter (if provided for) or is invalid. (Note that if + # there is a **kwargs parameter then any passed kwarg with a name + # that matches a positional-only argument will be considered valid and + # included to **kwargs, for example it's valid to call function + # `def g(a, /, **kwargs: int)` with `g(1, a=2, b=3)`, in which case + # **kwags will be received as `{'a': 2, 'b': 3}` - this is the standard + # Python behaviour.) + # Popping from kwargs necessary here to distinguish between a positional + # argument passed positionally and a keyword argument being passed with + # the same name (to be absorbed by **kwargs or to be considered invalid). + # When later in the code all parameters (positional and kwarg) are + # consoliated to `params_as_kwargs` that dict will include all + # positional-only arguments but it will not include any keyword arguments + # with the same name as a positional-only argument - such keyword + # arguments are instead held in this `kwarg_name_as_posonly` dict. (Why? + # Because they both have the same name - they can't both be represented + # in the same dictionary.) + kwarg_name_as_posonly = { + name: kwargs.pop(name) for name in posonly_args if name in kwargs + } + + # hint for any **kwargs parameter (None if no **kwargs or **kwargs not typed) + varkw_hint: type[Any] | typing._Final | None extra_kwargs = [a for a in kwargs if a not in all_param_names] if spec.varkw is None: # no provision for extra kwargs, e.g. no **kwargs in sig + varkw_hint = None excess_kwargs = extra_kwargs for name in excess_kwargs: del kwargs[name] extra_kwargs = [] + # no **kwargs to absorb positional-only inputs received by keyword + posonly_kwarg_errors = list(kwarg_name_as_posonly) + kwarg_name_as_posonly = {} else: # extra kwargs provided for, e.g. with **kwargs excess_kwargs = [] + # positional-only inputs received by keyword are absorbed by **kwargs + posonly_kwarg_errors = [] # add a hint for each extra kwarg - if hint := hints.get(spec.varkw, False): + varkw_hint = hints.get(spec.varkw) + if varkw_hint is not None: for name in extra_kwargs: - hints_[name] = hint + hints_[name] = varkw_hint del hints_[spec.varkw] sig_errors = validate_against_signature( - args_as_kwargs, kwargs, req_args, req_kwargs, excess_args, excess_kwargs + args_as_kwargs, + kwargs, + req_args, + req_kwargs, + excess_args, + excess_kwargs, + posonly_args, + posonly_kwarg_errors, ) - params_as_kwargs = { # remove arguments not provided for in signature + # remove arguments not provided for in signature + params_as_kwargs = { k: v for k, v in (args_as_kwargs | kwargs).items() if (k in all_param_names + extra_kwargs) or k.startswith(name_extra_args) } + # note that `kwarg_name_as_posonly` will not included to the above + # `params_as_kwargs` - see comment further above for `kwarg_name_as_posonly`. ann_errors = validate_against_hints( params_as_kwargs, hints_, no_item_validation=no_item_validation ) + # now consider any inputs absorbed by **kwargs that shared the same + # name as a positional-only argument. Validate these against the + # variadic type hint for **kwargs + if kwarg_name_as_posonly and varkw_hint is not None: + posonly_errors = validate_against_hints( + kwarg_name_as_posonly, + dict.fromkeys(kwarg_name_as_posonly, varkw_hint), + no_item_validation=no_item_validation, + ) + # disambiguate names that refer to keyword arguments received + # to **kwargs from positional-only args for the same name. + for name, error in posonly_errors.items(): + key = f"{name} (**{spec.varkw})" + ann_errors[key] = error + if sig_errors or ann_errors: raise InputsError(f.__name__, sig_errors, ann_errors) @@ -1098,27 +1238,12 @@ def wrapped_f(*args, **kwargs) -> Any: # noqa: C901, PLR0912 ) new_extra_args = [] - new_kwargs = {} + new_kwargs: dict[str, Any] = {} for name, obj in all_as_kwargs.items(): - if name not in hints_: - if name.startswith(name_extra_args): - new_extra_args.append(obj) - else: - new_kwargs[name] = obj - continue - hint = hints_[name] - if is_annotated(hint): - meta = hint.__metadata__ - for data in meta: - # let order of coercion and parsing depend on their - # order within metadata - if obj is not None and isinstance(data, Coerce): - obj = data.coerce_to(obj) # noqa: PLW2901 - if isinstance(data, Parser): - if obj is None and not data.parse_none: - continue - obj = data.function(name, obj, new_kwargs.copy()) # noqa: PLW2901 - + if name in hints_: + obj = apply_coerce_and_parser( # noqa: PLW2901 + name, obj, hints_[name], new_kwargs.copy() + ) if name.startswith(name_extra_args): new_extra_args.append(obj) else: @@ -1129,6 +1254,15 @@ def wrapped_f(*args, **kwargs) -> Any: # noqa: C901, PLR0912 new_args.append(new_kwargs[arg_name]) del new_kwargs[arg_name] + # add inputs absorbed by **kwargs that shared the same name as a + # positional-only argument. + for name, obj in kwarg_name_as_posonly.items(): + if varkw_hint is not None: + obj = apply_coerce_and_parser( # noqa: PLW2901 + name, obj, varkw_hint, new_kwargs.copy() + ) + new_kwargs[name] = obj + return f(*new_args, *new_extra_args, **new_kwargs) return wrapped_f diff --git a/tests/test_valimp.py b/tests/test_valimp.py index 8ff5f23..5cebb00 100644 --- a/tests/test_valimp.py +++ b/tests/test_valimp.py @@ -2214,3 +2214,221 @@ class DataCls: ) with pytest.raises(m.InputsError, match=re.escape(msg)): DataCls("not a list", {0: "val0"}) + + +def test_positional_only_valid(): + """Verify positional-only arguments can be passed positionally.""" + + @m.parse + def f( + a: Annotated[Union[int, str], m.Coerce(str)], + b: Annotated[int, m.Parser(lambda _n, obj, _p: obj + 1)], + /, + c: int, + d: int = 3, + ) -> tuple: + return a, b, c, d + + # all positional-only args passed positionally, including coerce/parse + assert f(5, 10, 2) == ("5", 11, 2, 3) + assert f(5, 10, 2, 4) == ("5", 11, 2, 4) + # the positional-or-keyword args can still be passed by keyword + assert f(5, 10, c=2, d=4) == ("5", 11, 2, 4) + + +def test_positional_only_as_keyword_invalid(): + """Verify error if positional-only argument passed as keyword. + + Verifies for a function that does not provide for **kwargs. + """ + + @m.parse + def f(a: int, /, b: str) -> tuple: + return a, b + + # single positional-only argument passed as keyword + msg = ( + "Inputs to 'f' do not conform with the function signature:" + "\n\nGot positional-only argument as keyword argument (and" + " signature makes no provision for **kwargs that would otherwise" + " receive it): 'a'." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + f(a=1, b="x") + + @m.parse + def g(a: int, b: str, /, c: int) -> tuple: + return a, b, c + + # multiple positional-only arguments passed as keyword + msg = ( + "Inputs to 'g' do not conform with the function signature:" + "\n\nGot positional-only arguments as keyword arguments (and" + " signature makes no provision for **kwargs that would otherwise" + " receive them): 'a' and 'b'." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + g(a=1, b="x", c=3) + + +def test_positional_only_as_keyword_with_other_sig_errors(): + """Verify positional-only error reported alongside other errors.""" + + @m.parse + def f(a: int, b: str, /, c: int, d: str = "hi", *, e: bool = False) -> tuple: + return a, b, c + + # 'a' passed positionally, as required + # 'b' passed as keyword (positional-only) + # 'c' missing and an unexpected keyword argument received. + msg = ( + "Inputs to 'f' do not conform with the function signature:" + "\n\nGot unexpected keyword argument: 'not_a_kwarg'." + "\n\nGot positional-only argument as keyword argument (and" + " signature makes no provision for **kwargs that would otherwise" + " receive it): 'b'." + "\n\nMissing 1 positional argument: 'c'." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + f(1, b="x", not_a_kwarg="foo") + + +def test_positional_only_absorbed_by_kwargs(): + """Verify positional-only name as keyword is absorbed by **kwargs. + + Matches standard Python behaviour where a keyword argument matching a + positional-only parameter name is absorbed by **kwargs rather than + binding to the parameter. + """ + + @m.parse + def f(a: int, /, **kwargs: int) -> tuple: + return a, kwargs + + # 'a' passed positionally, 'a' as keyword absorbed by **kwargs + assert f(1, a=2, b=3) == (1, {"a": 2, "b": 3}) + # only positional 'a' + assert f(5, x=9) == (5, {"x": 9}) + + +def test_positional_only_absorbed_by_kwargs_default(): + """Verify absorption by **kwargs when positional-only arg takes default.""" + + @m.parse + def f(a: str, b: int = 0, /, **kwargs: int) -> tuple: + return a, b, kwargs + + # 'b' not passed positionally so takes default, keyword 'b' to **kwargs + assert f("a", b=3, x=2) == ("a", 0, {"b": 3, "x": 2}) + + +def test_positional_only_required_missing_with_kwargs(): + """Verify required positional-only arg flagged missing despite **kwargs. + + A keyword matching the positional-only parameter name is absorbed by + **kwargs and does not satisfy the (required) positional-only parameter. + """ + + @m.parse + def f(a: int, /, **kwargs: int) -> tuple: + return a, kwargs + + msg = ( + "Inputs to 'f' do not conform with the function signature:" + "\n\nMissing 1 positional argument: 'a'." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + f(a=2, b=3) + + +def test_positional_only_absorbed_kwargs_validated(): + """Verify values absorbed by **kwargs are validated and coerced. + + Includes verification for a value absorbed under a positional-only + parameter name. + """ + + @m.parse + def f( + a: int, + /, + **kwargs: Annotated[Union[int, str], m.Coerce(str)], + ) -> tuple: + return a, kwargs + + # keyword 'a' absorbed to **kwargs and coerced to str + # keyword 'b' coerced to str against the **kwargs hint + assert f(1, a=2, b=3) == (1, {"a": "2", "b": "3"}) + + # absorbed value that does not conform with the **kwargs hint + @m.parse + def g(a: str, /, **kwargs: Union[list, tuple]) -> tuple: + return a, kwargs + + msg = ( + "The following inputs to 'g' do not conform with the corresponding" + " type annotation:\n\na (**kwargs)\n\tTakes input that conforms with" + " <(, )>" + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + g("1", a="not an int") + + +def test_positional_only_absorbed_kwargs_name_collision_errors(): + """Verify both errors reported when a colliding name is doubly invalid. + + When a keyword argument is received with the same name as a + positional-only argument, and both values are invalid against their + their respective type annotations (with the kwarg being absorbed within + **kwargs), both errors should be reported. + """ + + @m.parse + def f(a: str, /, **kwargs: int) -> tuple: + return a, kwargs + + # positional 'a' (1) invalid against `str`, absorbed 'a' ("x") invalid + # against the **kwargs hint `int` - both errors reported + msg = ( + "The following inputs to 'f' do not conform with the corresponding" + " type annotation:" + "\n\na\n\tTakes type although received '1' of type" + " ." + "\n\na (**kwargs)\n\tTakes type although received 'x'" + " of type ." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + f(1, a="x") + + # only the absorbed value is invalid (positional 'a' valid) + msg = ( + "The following inputs to 'f' do not conform with the corresponding" + " type annotation:\n\na (**kwargs)\n\tTakes type " + " although received 'x' of type ." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + f("ok", a="x") + + +def test_positional_only_method(): + """Verify positional-only support for a decorated method.""" + + class A: + """Class to hold decorated method.""" + + @m.parse + def meth(self, a: int, b: int, /, c: int) -> tuple: + return a, b, c + + inst = A() + assert inst.meth(1, 2, 3) == (1, 2, 3) + assert inst.meth(1, 2, c=3) == (1, 2, 3) + + msg = ( + "Inputs to 'meth' do not conform with the function signature:" + "\n\nGot positional-only argument as keyword argument (and" + " signature makes no provision for **kwargs that would otherwise" + " receive it): 'b'." + ) + with pytest.raises(m.InputsError, match=re.escape(msg)): + inst.meth(1, b=2, c=3)