From f2baf1ac7908d6c092f8a72a2191f6efb946114e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 10 Apr 2023 00:37:58 -0400 Subject: [PATCH 01/13] gh-87106: Fix `inspect.signature.bind` handling of positional-only arguments with `**kwargs` --- Lib/inspect.py | 26 +++++++++--------- Lib/test/test_inspect.py | 27 +++++++++---------- ...3-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst | 3 +++ 3 files changed, 29 insertions(+), 27 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst diff --git a/Lib/inspect.py b/Lib/inspect.py index 4242b40c2a08df..08370557ed109a 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3109,6 +3109,8 @@ def _bind(self, args, kwargs, *, partial=False): parameters_ex = () arg_vals = iter(args) + pos_only_param_in_kwargs = [] + while True: # Let's iterate through the positional arguments and corresponding # parameters @@ -3129,10 +3131,9 @@ def _bind(self, args, kwargs, *, partial=False): break elif param.name in kwargs: if param.kind == _POSITIONAL_ONLY: - msg = '{arg!r} parameter is positional only, ' \ - 'but was passed as a keyword' - msg = msg.format(arg=param.name) - raise TypeError(msg) from None + # Raise a TypeError once we are sure there is no + # **kwargs param later. + pos_only_param_in_kwargs.append(param) parameters_ex = (param,) break elif (param.kind == _VAR_KEYWORD or @@ -3215,20 +3216,21 @@ def _bind(self, args, kwargs, *, partial=False): else: if param.kind == _POSITIONAL_ONLY: - # This should never happen in case of a properly built - # Signature object (but let's have this check here - # to ensure correct behaviour just in case) - raise TypeError('{arg!r} parameter is positional only, ' - 'but was passed as a keyword'. \ - format(arg=param.name)) - - arguments[param_name] = arg_val + # Restore the param in case there is a kwargs_param + kwargs[param_name] = arg_val + else: + arguments[param_name] = arg_val if kwargs: if kwargs_param is not None: # Process our '**kwargs'-like parameter arguments[kwargs_param.name] = kwargs else: + if pos_only_param_in_kwargs: + raise TypeError('got some positional-only arguments passed as ' + 'keyword arguments: {arg!r} '. \ + format(arg=', '.join( + param.name for param in pos_only_param_in_kwargs))) raise TypeError( 'got an unexpected keyword argument {arg!r}'.format( arg=next(iter(kwargs)))) diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index 3a3646f1861e80..95bf205f7cc189 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -4155,18 +4155,9 @@ def test(a, *args, b, z=100, **kwargs): self.assertEqual(ba.args, (10, 20)) def test_signature_bind_positional_only(self): - P = inspect.Parameter - - def test(a_po, b_po, c_po=3, foo=42, *, bar=50, **kwargs): + def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): return a_po, b_po, c_po, foo, bar, kwargs - sig = inspect.signature(test) - new_params = collections.OrderedDict(tuple(sig.parameters.items())) - for name in ('a_po', 'b_po', 'c_po'): - new_params[name] = new_params[name].replace(kind=P.POSITIONAL_ONLY) - new_sig = sig.replace(parameters=new_params.values()) - test.__signature__ = new_sig - self.assertEqual(self.call(test, 1, 2, 4, 5, bar=6), (1, 2, 4, 5, 6, {})) @@ -4176,15 +4167,21 @@ def test(a_po, b_po, c_po=3, foo=42, *, bar=50, **kwargs): self.assertEqual(self.call(test, 1, 2, foo=4, bar=5), (1, 2, 3, 4, 5, {})) - with self.assertRaisesRegex(TypeError, "but was passed as a keyword"): - self.call(test, 1, 2, foo=4, bar=5, c_po=10) + self.assertEqual(self.call(test, 1, 2, foo=4, bar=5, c_po=10), + (1, 2, 3, 4, 5, {'c_po': 10})) - with self.assertRaisesRegex(TypeError, "parameter is positional only"): - self.call(test, 1, 2, c_po=4) + self.assertEqual(self.call(test, 1, 2, c_po=4), + (1, 2, 3, 42, 50, {'c_po': 4})) - with self.assertRaisesRegex(TypeError, "parameter is positional only"): + with self.assertRaisesRegex(TypeError, "missing 2 required positional arguments"): self.call(test, a_po=1, b_po=2) + def test_without_var_kwargs(a_po, b_po, c_po=3, /, foo=42, *, bar=50): + return a_po, b_po, c_po, foo, bar + + with self.assertRaisesRegex(TypeError, "positional-only arguments passed as keyword"): + self.call(test_without_var_kwargs, 1, 2, foo=4, bar=5, c_po=10) + def test_signature_bind_with_self_arg(self): # Issue #17071: one of the parameters is named "self def test(a, self, b): diff --git a/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst b/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst new file mode 100644 index 00000000000000..b76838a07229da --- /dev/null +++ b/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst @@ -0,0 +1,3 @@ +Fixed handling in :meth:`inspect.signature.bind` of keyword arguments having +the same name as positional-only arguments when a variadic keyword argument +(e.g. `**kwargs`) is present. From 487e2178b48e1cd21762e5d0b8440c663d47fc29 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Mon, 10 Apr 2023 00:51:19 -0400 Subject: [PATCH 02/13] fixup: double backticks --- .../next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst b/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst index b76838a07229da..6f13188f6f119f 100644 --- a/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst +++ b/Misc/NEWS.d/next/Library/2023-04-10-00-04-37.gh-issue-87106.UyBnPQ.rst @@ -1,3 +1,3 @@ Fixed handling in :meth:`inspect.signature.bind` of keyword arguments having the same name as positional-only arguments when a variadic keyword argument -(e.g. `**kwargs`) is present. +(e.g. ``**kwargs``) is present. From b7cbfe4a05aa0a682b6731e32cb734931d6ea5a5 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 14 Apr 2023 22:58:38 -0400 Subject: [PATCH 03/13] Avoid having helper function start with "test" --- Lib/test/test_inspect.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_inspect.py b/Lib/test/test_inspect.py index 0f819c3bb6e5ea..fd89bd981c0521 100644 --- a/Lib/test/test_inspect.py +++ b/Lib/test/test_inspect.py @@ -4174,11 +4174,11 @@ def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): with self.assertRaisesRegex(TypeError, "missing 2 required positional arguments"): self.call(test, a_po=1, b_po=2) - def test_without_var_kwargs(a_po, b_po, c_po=3, /, foo=42, *, bar=50): + def without_var_kwargs(a_po, b_po, c_po=3, /, foo=42, *, bar=50): return a_po, b_po, c_po, foo, bar with self.assertRaisesRegex(TypeError, "positional-only arguments passed as keyword"): - self.call(test_without_var_kwargs, 1, 2, foo=4, bar=5, c_po=10) + self.call(without_var_kwargs, 1, 2, foo=4, bar=5, c_po=10) def test_signature_bind_with_self_arg(self): # Issue #17071: one of the parameters is named "self From cc2d7ba67637414729e764443ee79edd9f05d2b9 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 09:37:38 -0400 Subject: [PATCH 04/13] Opt for black style Co-authored-by: Nikita Sobolev --- Lib/inspect.py | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 37887fb1091bcf..94d30263cff0a8 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3297,10 +3297,15 @@ def _bind(self, args, kwargs, *, partial=False): arguments[kwargs_param.name] = kwargs else: if pos_only_param_in_kwargs: - raise TypeError('got some positional-only arguments passed as ' - 'keyword arguments: {arg!r} '. \ - format(arg=', '.join( - param.name for param in pos_only_param_in_kwargs))) + raise TypeError( + 'got some positional-only arguments passed as ' + 'keyword arguments: {arg!r} '.format( + arg=', '.join( + param.name + for param in pos_only_param_in_kwargs + ), + ), + ) raise TypeError( 'got an unexpected keyword argument {arg!r}'.format( arg=next(iter(kwargs)))) From 366bb96d7617d8fa9bd8566d206b7af022ad98cf Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 09:49:44 -0400 Subject: [PATCH 05/13] Simplify test case --- Lib/test/test_inspect/test_inspect.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index d455251f226112..d3a755e816741f 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -5081,11 +5081,11 @@ def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): with self.assertRaisesRegex(TypeError, "missing 2 required positional arguments"): self.call(test, a_po=1, b_po=2) - def without_var_kwargs(a_po, b_po, c_po=3, /, foo=42, *, bar=50): - return a_po, b_po, c_po, foo, bar + def without_var_kwargs(c_po=3, /): + return c_po with self.assertRaisesRegex(TypeError, "positional-only arguments passed as keyword"): - self.call(without_var_kwargs, 1, 2, foo=4, bar=5, c_po=10) + self.call(without_var_kwargs, c_po=10) def test_signature_bind_with_self_arg(self): # Issue #17071: one of the parameters is named "self From 2f792eb04bd600e731d034fab513cd45b0470767 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 09:53:25 -0400 Subject: [PATCH 06/13] Add test cases --- Lib/test/test_inspect/test_inspect.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index d3a755e816741f..cb572aebf3e7a1 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -5075,6 +5075,12 @@ def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): self.assertEqual(self.call(test, 1, 2, foo=4, bar=5, c_po=10), (1, 2, 3, 4, 5, {'c_po': 10})) + self.assertEqual(self.call(test, 1, 2, 3, c_po=10, foo=4, bar=5), + (1, 2, 3, 4, 5, {'c_po': 10})) + + self.assertEqual(self.call(test, 1, 2, 3, foo=4, bar=5, c_po=10), + (1, 2, 3, 4, 5, {'c_po': 10})) + self.assertEqual(self.call(test, 1, 2, c_po=4), (1, 2, 3, 42, 50, {'c_po': 4})) From 5951ad901e2dbb456cadf0c863b3f29ba78eae7e Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 10:05:02 -0400 Subject: [PATCH 07/13] Use elif --- Lib/inspect.py | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 94d30263cff0a8..28837ec701976a 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3295,20 +3295,21 @@ def _bind(self, args, kwargs, *, partial=False): if kwargs_param is not None: # Process our '**kwargs'-like parameter arguments[kwargs_param.name] = kwargs - else: - if pos_only_param_in_kwargs: - raise TypeError( - 'got some positional-only arguments passed as ' - 'keyword arguments: {arg!r} '.format( - arg=', '.join( - param.name - for param in pos_only_param_in_kwargs - ), + elif pos_only_param_in_kwargs: + raise TypeError( + 'got some positional-only arguments passed as ' + 'keyword arguments: {arg!r} '.format( + arg=', '.join( + param.name + for param in pos_only_param_in_kwargs ), - ) + ), + ) + else: raise TypeError( 'got an unexpected keyword argument {arg!r}'.format( - arg=next(iter(kwargs)))) + arg=next(iter(kwargs))) + ) return self._bound_arguments_cls(self, arguments) From 2ecde833fe273703ff40573491099f5cb544626d Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 10:31:22 -0400 Subject: [PATCH 08/13] Refactor to avoid pop-and-maybe-put-back pattern --- Lib/inspect.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 28837ec701976a..5d41839054aca6 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3271,6 +3271,9 @@ def _bind(self, args, kwargs, *, partial=False): # before reaching the last parameter before *args. continue + if param.kind == _POSITIONAL_ONLY: + continue + param_name = param.name try: arg_val = kwargs.pop(param_name) @@ -3283,13 +3286,8 @@ def _bind(self, args, kwargs, *, partial=False): param.default is _empty): raise TypeError('missing a required argument: {arg!r}'. \ format(arg=param_name)) from None - else: - if param.kind == _POSITIONAL_ONLY: - # Restore the param in case there is a kwargs_param - kwargs[param_name] = arg_val - else: - arguments[param_name] = arg_val + arguments[param_name] = arg_val if kwargs: if kwargs_param is not None: From db2d92595310b115bc98dedc7f14c502e114b2ea Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 10:32:40 -0400 Subject: [PATCH 09/13] Remove unnecessary cosmetic changes --- Lib/inspect.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 5d41839054aca6..fe64949e4fe80f 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3286,6 +3286,7 @@ def _bind(self, args, kwargs, *, partial=False): param.default is _empty): raise TypeError('missing a required argument: {arg!r}'. \ format(arg=param_name)) from None + else: arguments[param_name] = arg_val @@ -3306,8 +3307,7 @@ def _bind(self, args, kwargs, *, partial=False): else: raise TypeError( 'got an unexpected keyword argument {arg!r}'.format( - arg=next(iter(kwargs))) - ) + arg=next(iter(kwargs)))) return self._bound_arguments_cls(self, arguments) From 69162908a945b998b62a39af9089c6feb8e6b4ff Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 5 May 2024 10:57:58 -0400 Subject: [PATCH 10/13] Test multiple pos-only args passed by keyword --- Lib/inspect.py | 3 ++- Lib/test/test_inspect/test_inspect.py | 11 +++++++---- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index fe64949e4fe80f..46fa0b45eb9a35 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3204,6 +3204,7 @@ def _bind(self, args, kwargs, *, partial=False): # Raise a TypeError once we are sure there is no # **kwargs param later. pos_only_param_in_kwargs.append(param) + continue parameters_ex = (param,) break elif (param.kind == _VAR_KEYWORD or @@ -3297,7 +3298,7 @@ def _bind(self, args, kwargs, *, partial=False): elif pos_only_param_in_kwargs: raise TypeError( 'got some positional-only arguments passed as ' - 'keyword arguments: {arg!r} '.format( + 'keyword arguments: {arg!r}'.format( arg=', '.join( param.name for param in pos_only_param_in_kwargs diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index cb572aebf3e7a1..f53069edf164cb 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -5087,11 +5087,14 @@ def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): with self.assertRaisesRegex(TypeError, "missing 2 required positional arguments"): self.call(test, a_po=1, b_po=2) - def without_var_kwargs(c_po=3, /): - return c_po + def without_var_kwargs(c_po=3, d_po=4, /): + return c_po, d_po - with self.assertRaisesRegex(TypeError, "positional-only arguments passed as keyword"): - self.call(without_var_kwargs, c_po=10) + with self.assertRaisesRegex( + TypeError, + "positional-only arguments passed as keyword arguments: 'c_po, d_po'", + ): + self.call(without_var_kwargs, c_po=33, d_po=44) def test_signature_bind_with_self_arg(self): # Issue #17071: one of the parameters is named "self From 1ec398b70028723d0b47b5f6cf958c5f11cc9c65 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 10 May 2024 08:27:24 -0400 Subject: [PATCH 11/13] Remove superfluous continue --- Lib/inspect.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/Lib/inspect.py b/Lib/inspect.py index 46fa0b45eb9a35..9c983e861b101d 100644 --- a/Lib/inspect.py +++ b/Lib/inspect.py @@ -3272,9 +3272,6 @@ def _bind(self, args, kwargs, *, partial=False): # before reaching the last parameter before *args. continue - if param.kind == _POSITIONAL_ONLY: - continue - param_name = param.name try: arg_val = kwargs.pop(param_name) From f1565de5658f3beeef5784966055c780291b231c Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Fri, 10 May 2024 08:29:18 -0400 Subject: [PATCH 12/13] [tests] Use distinct value for c_po kwarg --- Lib/test/test_inspect/test_inspect.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index f53069edf164cb..097a3e837384ad 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -5075,11 +5075,11 @@ def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): self.assertEqual(self.call(test, 1, 2, foo=4, bar=5, c_po=10), (1, 2, 3, 4, 5, {'c_po': 10})) - self.assertEqual(self.call(test, 1, 2, 3, c_po=10, foo=4, bar=5), - (1, 2, 3, 4, 5, {'c_po': 10})) + self.assertEqual(self.call(test, 1, 2, 3, c_po=30, foo=4, bar=5), + (1, 2, 3, 4, 5, {'c_po': 30})) - self.assertEqual(self.call(test, 1, 2, 3, foo=4, bar=5, c_po=10), - (1, 2, 3, 4, 5, {'c_po': 10})) + self.assertEqual(self.call(test, 1, 2, 3, foo=4, bar=5, c_po=30), + (1, 2, 3, 4, 5, {'c_po': 30})) self.assertEqual(self.call(test, 1, 2, c_po=4), (1, 2, 3, 42, 50, {'c_po': 4})) From 52facb0a9d96125ddd5b0f0d67912a6a8fe84ba5 Mon Sep 17 00:00:00 2001 From: Jacob Walls Date: Sun, 12 May 2024 09:40:30 -0400 Subject: [PATCH 13/13] fixup! [tests] Use distinct value for c_po kwarg --- Lib/test/test_inspect/test_inspect.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Lib/test/test_inspect/test_inspect.py b/Lib/test/test_inspect/test_inspect.py index 097a3e837384ad..5aa3de45d70a08 100644 --- a/Lib/test/test_inspect/test_inspect.py +++ b/Lib/test/test_inspect/test_inspect.py @@ -5075,11 +5075,11 @@ def test(a_po, b_po, c_po=3, /, foo=42, *, bar=50, **kwargs): self.assertEqual(self.call(test, 1, 2, foo=4, bar=5, c_po=10), (1, 2, 3, 4, 5, {'c_po': 10})) - self.assertEqual(self.call(test, 1, 2, 3, c_po=30, foo=4, bar=5), - (1, 2, 3, 4, 5, {'c_po': 30})) + self.assertEqual(self.call(test, 1, 2, 30, c_po=31, foo=4, bar=5), + (1, 2, 30, 4, 5, {'c_po': 31})) - self.assertEqual(self.call(test, 1, 2, 3, foo=4, bar=5, c_po=30), - (1, 2, 3, 4, 5, {'c_po': 30})) + self.assertEqual(self.call(test, 1, 2, 30, foo=4, bar=5, c_po=31), + (1, 2, 30, 4, 5, {'c_po': 31})) self.assertEqual(self.call(test, 1, 2, c_po=4), (1, 2, 3, 42, 50, {'c_po': 4}))