Skip to content

gh-103556: [inspect.Signature] disallow pos-or-kw params without default after pos-only with default #103557

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
carljm merged 4 commits into from
Apr 22, 2023
Merged

gh-103556: [inspect.Signature] disallow pos-or-kw params without default after pos-only with default #103557

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
578f998
gh-103556: [inspect.Signature] disallow pos-or-kw params without defa…
sobolevn Apr 15, 2023
a4cf538
Update Lib/test/test_inspect.py
sobolevn Apr 15, 2023
d6ef93a
Address review
sobolevn Apr 19, 2023
261d9d1
Merge branch 'issue-103556' of https://github.com/sobolevn/cpython in…
sobolevn Apr 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Next Next commit
gh-103556: [inspect.Signature] disallow pos-or-kw params without defa… 
…ult after pos-only with default
  • Loading branch information
@sobolevn
sobolevn committed Apr 15, 2023
commit 578f998916cfbc97f14829b7f841886f67b198b4
7 changes: 6 additions & 1 deletion Lib/inspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3021,7 +3021,12 @@ def __init__(self, parameters=None, *, return_annotation=_empty,
kind.description)
raise ValueError(msg)
elif kind > top_kind:
kind_defaults = False
if (top_kind != _POSITIONAL_ONLY
and kind != _POSITIONAL_OR_KEYWORD):
# We still have to maintain defaults in cases like
# def some(pod=42, /, pk=1): ...
# Here `pk` must have a default value.
kind_defaults = False

@carljm carljm Apr 17, 2023

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-default-after-default is only checked for _POSITIONAL_ONLY and _POSITIONAL_KEYWORD (see line 3032.) So if we want to keep the value of kind_defaults across the transition from PO to POK (which I do agree is correct and matches the parser behavior), then really we always want to keep it, and it is no longer kind_defaults but simply seen_default. So I think we can eliminate all of this, here's the full version I'd recommend (which passes all tests): https://gist.github.com/carljm/6779e601d04e0b1135444b4e0263e281

@sobolevn sobolevn Apr 19, 2023

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I agree, thank you for the suggestion.

top_kind = kind

if kind in (_POSITIONAL_ONLY, _POSITIONAL_OR_KEYWORD):
Expand Down
40 changes: 34 additions & 6 deletions Lib/test/test_inspect.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2462,18 +2462,43 @@ def test_signature_object(self):
self.assertEqual(str(S()), '()')
self.assertEqual(repr(S().parameters), 'mappingproxy(OrderedDict())')

def test(po, pk, pod=42, pkd=100, *args, ko, **kwargs):
def test(po, /, pk, pkd=100, *args, ko, kod=10, **kwargs):
pass

sig = inspect.signature(test)
po = sig.parameters['po'].replace(kind=P.POSITIONAL_ONLY)
pod = sig.parameters['pod'].replace(kind=P.POSITIONAL_ONLY)
self.assertTrue(repr(sig).startswith('<Signature'))
self.assertTrue('(po, /, pk' in repr(sig))

# We need to functions, because it is impossible to represent
# all arg kinds in a single one.
Comment thread
sobolevn marked this conversation as resolved.
Outdated
def test2(pod=42, /):
pass

sig2 = inspect.signature(test2)
self.assertTrue(repr(sig2).startswith('<Signature'))
self.assertTrue('(pod=42, /)' in repr(sig2))

po = sig.parameters['po']
pod = sig2.parameters['pod']
pk = sig.parameters['pk']
pkd = sig.parameters['pkd']
args = sig.parameters['args']
ko = sig.parameters['ko']
kod = sig.parameters['kod']
kwargs = sig.parameters['kwargs']

S((po, pk, args, ko, kwargs))
S((po, pk, ko, kod))
S((po, pod, ko))
S((po, pod, kod))
S((pod, ko, kod))
S((pod, kod))
S((pod, args, kod, kwargs))
# keyword-only parameters without default values
# can follow keyword-only parameters with default values:
S((kod, ko))
S((kod, ko, kwargs))
S((args, kod, ko))

with self.assertRaisesRegex(ValueError, 'wrong parameter order'):
S((pk, po, args, ko, kwargs))
Expand All @@ -2494,15 +2519,18 @@ def test(po, pk, pod=42, pkd=100, *args, ko, **kwargs):
with self.assertRaisesRegex(ValueError, 'follows default argument'):
S((pod, po))

with self.assertRaisesRegex(ValueError, 'follows default argument'):
S((pod, pk))

with self.assertRaisesRegex(ValueError, 'follows default argument'):
S((po, pod, pk))

with self.assertRaisesRegex(ValueError, 'follows default argument'):
S((po, pkd, pk))

with self.assertRaisesRegex(ValueError, 'follows default argument'):
S((pkd, pk))

self.assertTrue(repr(sig).startswith('<Signature'))
self.assertTrue('(po, pk' in repr(sig))

def test_signature_object_pickle(self):
def foo(a, b, *, c:1={}, **kw) -> {42:'ham'}: pass
foo_partial = functools.partial(foo, a=1)
Expand Down
3 changes: 3 additions & 0 deletions Misc/NEWS.d/next/Library/2023-04-15-12-19-14.gh-issue-103556.TEf-2m.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Now creating :class:`inspect.Signature` objects with positional-only
parameter with a default followed by a positional-or-keyword parameter
without one is impossible.