From 34d589380a6082e28f86657555d8e13635427863 Mon Sep 17 00:00:00 2001 From: lzrpotato Date: Sat, 1 Jul 2023 15:49:16 -0600 Subject: [PATCH 1/6] fix_issue #269 --- test/test_decorator.py | 25 +++---------------------- 1 file changed, 3 insertions(+), 22 deletions(-) diff --git a/test/test_decorator.py b/test/test_decorator.py index d70811d2..2e0b8eee 100644 --- a/test/test_decorator.py +++ b/test/test_decorator.py @@ -44,27 +44,8 @@ def pop(self, index, default_value=None): def partial(fn: Callable, *args, **kwargs) -> Callable: - """Partial via changing the signature defaults.""" - - @functools.wraps(fn) - def _wrapper(*other_args, **other_kwargs): - return fn(*other_args, **other_kwargs) - - signature = inspect.signature(fn) - parameters = signature.parameters - - args = ListWithPopDefault(args) - - new_parameters = [] - for key, param in parameters.items(): - param = typing.cast(inspect.Parameter, param) - default = args.pop(0, None) or kwargs.get(key, None) - if default is not None: - param = param.replace(default=default) - new_parameters.append(param) - - signature = signature.replace(parameters=new_parameters) - _wrapper.__signature__ = signature + _wrapper = functools.partial(fn, *args, **kwargs) + _wrapper.__qualname__ = fn.__qualname__ return _wrapper @@ -73,7 +54,7 @@ def _wrapper(*other_args, **other_kwargs): "args, expected, fn", [ ("", 1, partial(_fn_with_positional_only, 1)), - ("2", 2, partial(_fn_with_positional_only, 1)), + ("2", 2, partial(_fn_with_positional_only)), ("2", 2, _fn_with_positional_only), ("", 1, partial(_fn_with_keyword_only, x=1)), ("--x=2", 2, partial(_fn_with_keyword_only, x=1)), From aac7ee25b03d133c29c597ef1588c0c86f70a075 Mon Sep 17 00:00:00 2001 From: lzrpotato Date: Sat, 1 Jul 2023 15:49:45 -0600 Subject: [PATCH 2/6] apply lint check --- test/test_decorator.py | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/test/test_decorator.py b/test/test_decorator.py index 2e0b8eee..a6a49b57 100644 --- a/test/test_decorator.py +++ b/test/test_decorator.py @@ -1,11 +1,8 @@ -"""Tests simple parsing decorators. -""" +"""Tests simple parsing decorators.""" import collections import dataclasses import functools -import inspect import sys -import typing from typing import Callable import pytest From cbb0c38364b70261f1250633575e9dd060d6eee3 Mon Sep 17 00:00:00 2001 From: lzrpotato Date: Sat, 1 Jul 2023 16:22:55 -0600 Subject: [PATCH 3/6] update --- test/test_decorator.py | 8 -------- 1 file changed, 8 deletions(-) diff --git a/test/test_decorator.py b/test/test_decorator.py index a6a49b57..03b46f77 100644 --- a/test/test_decorator.py +++ b/test/test_decorator.py @@ -32,14 +32,6 @@ def _fn_with_all_argument_types(a: int, /, b: int, *, c: int) -> int: return a + b + c -class ListWithPopDefault(collections.UserList): - def pop(self, index, default_value=None): - try: - return super().pop(index) - except IndexError: - return default_value - - def partial(fn: Callable, *args, **kwargs) -> Callable: _wrapper = functools.partial(fn, *args, **kwargs) _wrapper.__qualname__ = fn.__qualname__ From 1187951033c1c7d53d4d780567207e5d149e95b9 Mon Sep 17 00:00:00 2001 From: lzrpotato Date: Sun, 2 Jul 2023 13:47:09 -0600 Subject: [PATCH 4/6] update --- test/test_decorator.py | 82 +++++++++++++++++++++++++++++++----------- 1 file changed, 61 insertions(+), 21 deletions(-) diff --git a/test/test_decorator.py b/test/test_decorator.py index 03b46f77..1fcd1c6d 100644 --- a/test/test_decorator.py +++ b/test/test_decorator.py @@ -4,7 +4,8 @@ import functools import sys from typing import Callable - +import typing +import inspect import pytest import simple_parsing as sp @@ -32,36 +33,82 @@ def _fn_with_all_argument_types(a: int, /, b: int, *, c: int) -> int: return a + b + c +class ListWithPopDefault(collections.UserList): + def pop(self, index, default_value=None): + try: + return super().pop(index) + except IndexError: + return default_value + + +def delay_evaluation_wrapper(fn: Callable) -> Callable: + def delayed_call(*args, **kwargs): + return lambda : fn(*args, **kwargs) + return delayed_call + + +@delay_evaluation_wrapper def partial(fn: Callable, *args, **kwargs) -> Callable: - _wrapper = functools.partial(fn, *args, **kwargs) - _wrapper.__qualname__ = fn.__qualname__ + """Partial via changing the signature defaults.""" + + @functools.wraps(fn) + def _wrapper(*other_args, **other_kwargs): + return fn(*other_args, **other_kwargs) + + signature = inspect.signature(fn) + parameters = signature.parameters + + args = ListWithPopDefault(args) + + new_parameters = [] + for key, param in parameters.items(): + param = typing.cast(inspect.Parameter, param) + default = args.pop(0, None) or kwargs.get(key, None) + if default is not None: + param = param.replace(default=default) + new_parameters.append(param) + + signature = signature.replace(parameters=new_parameters) + _wrapper.__signature__ = signature return _wrapper +def _xfail_in_py311(*param): + return pytest.param( + *param, + marks=pytest.mark.xfail( + sys.version_info >= (3, 11), + reason="TODO: test doesn't work in Python 3.11", + strict=True, + ), + ) + + @pytest.mark.parametrize( - "args, expected, fn", + "args, expected, delay_wrapper", [ ("", 1, partial(_fn_with_positional_only, 1)), + ("2", 2, partial(_fn_with_positional_only, 1)), ("2", 2, partial(_fn_with_positional_only)), - ("2", 2, _fn_with_positional_only), ("", 1, partial(_fn_with_keyword_only, x=1)), ("--x=2", 2, partial(_fn_with_keyword_only, x=1)), - ("--x=2", 2, _fn_with_keyword_only), + ("--x=2", 2, partial(_fn_with_keyword_only)), ("", 3, partial(_fn_with_all_argument_types, 1, b=1, c=1)), ("2", 4, partial(_fn_with_all_argument_types, b=1, c=1)), ("2 --b=2", 5, partial(_fn_with_all_argument_types, c=1)), - ("2 --b=2 --c=2", 6, _fn_with_all_argument_types), + ("2 --b=2 --c=2", 6, partial(_fn_with_all_argument_types)), ("--c=2", 4, partial(_fn_with_all_argument_types, 1, b=1)), - ("--b=2", 4, partial(_fn_with_all_argument_types, 1, c=1)), - ("--b=2 --c=2", 5, partial(_fn_with_all_argument_types, 1)), + _xfail_in_py311("--b=2", 4, partial(_fn_with_all_argument_types, 1, c=1)), + _xfail_in_py311("--b=2 --c=2", 5, partial(_fn_with_all_argument_types, 1)), ], ) def test_simple_arguments( args: str, expected: int, - fn: Callable, + delay_wrapper: Callable, ): + fn = delay_wrapper() decorated = sp.decorators.main(fn, args=args) assert decorated() == expected @@ -70,19 +117,11 @@ def _fn_with_nested_dataclass(x: int, /, *, data: AddThreeNumbers) -> int: return x + data() -def _xfail_in_py311(*param): - return pytest.param( - *param, - marks=pytest.mark.xfail( - sys.version_info >= (3, 11), - reason="TODO: test doesn't work in Python 3.11", - strict=True, - ), - ) + @pytest.mark.parametrize( - "args, expected, fn", + "args, expected, delay_wrapper", [ _xfail_in_py311("", 1, partial(_fn_with_nested_dataclass, 1, data=AddThreeNumbers())), ("--a=1", 2, partial(_fn_with_nested_dataclass, 1)), @@ -94,7 +133,8 @@ def _xfail_in_py311(*param): def test_nested_dataclass( args: str, expected: int, - fn: Callable, + delay_wrapper: Callable, ): + fn = delay_wrapper() decorated = sp.decorators.main(fn, args=args) assert decorated() == expected From a9c075aa7abb60e08b8e7a64cf595b488a339f73 Mon Sep 17 00:00:00 2001 From: lzrpotato Date: Sun, 2 Jul 2023 13:47:57 -0600 Subject: [PATCH 5/6] apply lint check --- test/test_decorator.py | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/test/test_decorator.py b/test/test_decorator.py index 1fcd1c6d..b4060cd8 100644 --- a/test/test_decorator.py +++ b/test/test_decorator.py @@ -43,7 +43,8 @@ def pop(self, index, default_value=None): def delay_evaluation_wrapper(fn: Callable) -> Callable: def delayed_call(*args, **kwargs): - return lambda : fn(*args, **kwargs) + return lambda: fn(*args, **kwargs) + return delayed_call @@ -117,9 +118,6 @@ def _fn_with_nested_dataclass(x: int, /, *, data: AddThreeNumbers) -> int: return x + data() - - - @pytest.mark.parametrize( "args, expected, delay_wrapper", [ From b7afa6ef3c549d7875dc02bed21cae72513d594e Mon Sep 17 00:00:00 2001 From: Fabrice Normandin Date: Mon, 10 Jul 2023 16:48:18 -0400 Subject: [PATCH 6/6] Simplify `test_decorator` Signed-off-by: Fabrice Normandin --- test/test_decorator.py | 67 +++++++++++++++++++++--------------------- 1 file changed, 34 insertions(+), 33 deletions(-) diff --git a/test/test_decorator.py b/test/test_decorator.py index b4060cd8..9521eee7 100644 --- a/test/test_decorator.py +++ b/test/test_decorator.py @@ -41,15 +41,7 @@ def pop(self, index, default_value=None): return default_value -def delay_evaluation_wrapper(fn: Callable) -> Callable: - def delayed_call(*args, **kwargs): - return lambda: fn(*args, **kwargs) - - return delayed_call - - -@delay_evaluation_wrapper -def partial(fn: Callable, *args, **kwargs) -> Callable: +def change_defaults(fn: Callable, *args, **kwargs) -> Callable: """Partial via changing the signature defaults.""" @functools.wraps(fn) @@ -81,35 +73,43 @@ def _xfail_in_py311(*param): marks=pytest.mark.xfail( sys.version_info >= (3, 11), reason="TODO: test doesn't work in Python 3.11", + raises=ValueError, # "non-default argument follows default argument" strict=True, ), ) @pytest.mark.parametrize( - "args, expected, delay_wrapper", + "args, expected, fn", [ - ("", 1, partial(_fn_with_positional_only, 1)), - ("2", 2, partial(_fn_with_positional_only, 1)), - ("2", 2, partial(_fn_with_positional_only)), - ("", 1, partial(_fn_with_keyword_only, x=1)), - ("--x=2", 2, partial(_fn_with_keyword_only, x=1)), - ("--x=2", 2, partial(_fn_with_keyword_only)), - ("", 3, partial(_fn_with_all_argument_types, 1, b=1, c=1)), - ("2", 4, partial(_fn_with_all_argument_types, b=1, c=1)), - ("2 --b=2", 5, partial(_fn_with_all_argument_types, c=1)), - ("2 --b=2 --c=2", 6, partial(_fn_with_all_argument_types)), - ("--c=2", 4, partial(_fn_with_all_argument_types, 1, b=1)), - _xfail_in_py311("--b=2", 4, partial(_fn_with_all_argument_types, 1, c=1)), - _xfail_in_py311("--b=2 --c=2", 5, partial(_fn_with_all_argument_types, 1)), + ("", 1, change_defaults(_fn_with_positional_only, 1)), + ("2", 2, change_defaults(_fn_with_positional_only, 1)), + ("2", 2, change_defaults(_fn_with_positional_only)), + ("", 1, change_defaults(_fn_with_keyword_only, x=1)), + ("--x=2", 2, change_defaults(_fn_with_keyword_only, x=1)), + ("--x=2", 2, change_defaults(_fn_with_keyword_only)), + ("", 3, change_defaults(_fn_with_all_argument_types, 1, b=1, c=1)), + ("2", 4, change_defaults(_fn_with_all_argument_types, b=1, c=1)), + ("2 --b=2", 5, change_defaults(_fn_with_all_argument_types, c=1)), + ("2 --b=2 --c=2", 6, change_defaults(_fn_with_all_argument_types)), + ("--c=2", 4, change_defaults(_fn_with_all_argument_types, 1, b=1)), + _xfail_in_py311( + "--b=2", 4, functools.partial(change_defaults, _fn_with_all_argument_types, 1, c=1) + ), + _xfail_in_py311( + "--b=2 --c=2", 5, functools.partial(change_defaults, _fn_with_all_argument_types, 1) + ), ], ) def test_simple_arguments( args: str, expected: int, - delay_wrapper: Callable, + fn: Callable, ): - fn = delay_wrapper() + if isinstance(fn, functools.partial): + # In Python 3.11.4, the inspect module had a backward-incompatible change. We need to have + # this additional level of indirection. + fn = fn() decorated = sp.decorators.main(fn, args=args) assert decorated() == expected @@ -119,20 +119,21 @@ def _fn_with_nested_dataclass(x: int, /, *, data: AddThreeNumbers) -> int: @pytest.mark.parametrize( - "args, expected, delay_wrapper", + ("args", "expected", "fn"), [ - _xfail_in_py311("", 1, partial(_fn_with_nested_dataclass, 1, data=AddThreeNumbers())), - ("--a=1", 2, partial(_fn_with_nested_dataclass, 1)), - ("--a=1 --b=1", 3, partial(_fn_with_nested_dataclass, 1)), - ("--a=1 --b=1 --c=1", 4, partial(_fn_with_nested_dataclass, 1)), - ("2 --a=1 --b=1 --c=1", 5, partial(_fn_with_nested_dataclass)), + _xfail_in_py311( + "", 1, change_defaults(_fn_with_nested_dataclass, 1, data=AddThreeNumbers()) + ), + ("--a=1", 2, change_defaults(_fn_with_nested_dataclass, 1)), + ("--a=1 --b=1", 3, change_defaults(_fn_with_nested_dataclass, 1)), + ("--a=1 --b=1 --c=1", 4, change_defaults(_fn_with_nested_dataclass, 1)), + ("2 --a=1 --b=1 --c=1", 5, change_defaults(_fn_with_nested_dataclass)), ], ) def test_nested_dataclass( args: str, expected: int, - delay_wrapper: Callable, + fn: Callable, ): - fn = delay_wrapper() decorated = sp.decorators.main(fn, args=args) assert decorated() == expected