Skip to content
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.

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

Modernize test_inspect by adding real pos-only parameters #103406

Closed
sobolevn opened this issue Apr 10, 2023 · 2 comments
Closed

Modernize test_inspect by adding real pos-only parameters #103406

sobolevn opened this issue Apr 10, 2023 · 2 comments
Assignees
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Apr 10, 2023

Right now test_inspect uses several hacks to pretend that some parameters are positional only:

def test(po, pk, pod=42, pkd=100, *args, ko, **kwargs):
pass
sig = inspect.signature(test)
po = sig.parameters['po'].replace(kind=P.POSITIONAL_ONLY)
pod = sig.parameters['pod'].replace(kind=P.POSITIONAL_ONLY)

myparam = MyParameter(name='z', kind=inspect.Parameter.POSITIONAL_ONLY)
myparams = collections.OrderedDict(sig.parameters, a=myparam)
mysig = MySignature().replace(parameters=myparams.values(),
return_annotation=sig.return_annotation)

def foo(a, b, c, d, **kwargs):
pass
sig = inspect.signature(foo)
params = sig.parameters.copy()
params['a'] = params['a'].replace(kind=Parameter.POSITIONAL_ONLY)
params['b'] = params['b'].replace(kind=Parameter.POSITIONAL_ONLY)

def test(a_po, *, b, **kwargs):
return a_po, kwargs
sig = inspect.signature(test)
new_params = list(sig.parameters.values())
new_params[0] = new_params[0].replace(kind=P.POSITIONAL_ONLY)
test.__signature__ = sig.replace(parameters=new_params)

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)

And maybe others.

It makes code more complex, unclear, and hides the real purpose of these tests.

This is not a design decision, but rather a limitation at the time. Commits are quite old and pos-only syntax was not available 9 and 11 years ago:

So, I propose to simplify these tests and make them more correct by using explicit pos only parameters.

Plus, we can keep one test like this to be sure that chaning a parameter kind still works as before. But, there's no need in keeping these old tests the way they are.

I will send a PR with the fix 👍

Linked PRs

@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir labels Apr 10, 2023
@sobolevn sobolevn self-assigned this Apr 10, 2023
@sobolevn
Copy link
Member Author

Since all supported Python versions have pos-only parameters this can be backported to reduce the amount of potential future merge conflicts.

@sobolevn
Copy link
Member Author

Ok, some code samples that I've listed cannot be replaced, because they test different things. But, some - can be.

miss-islington pushed a commit to miss-islington/cpython that referenced this issue Apr 14, 2023
…e` (pythonGH-103407)

(cherry picked from commit 7569781)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
miss-islington added a commit that referenced this issue Apr 14, 2023
…-103407)

(cherry picked from commit 7569781)

Co-authored-by: Nikita Sobolev <mail@sobolevn.me>
carljm added a commit to carljm/cpython that referenced this issue Apr 14, 2023
* main:
  pythongh-103532: Remove TKINTER_PROTECT_LOADTK code (pythonGH-103535)
  pythongh-103180: Add CI timeouts to all GitHub Actions jobs (python#103437)
  Remove double space in import error message (python#103458)
  ipaddress: Remove non-existent ip_str param from docstring (python#103461)
  Fix syntax typo in isolating extensions doc (python#103516)
  pythongh-103406: Modernize pos-only arguments usage in `test_signature` (python#103407)
  Proofread howto/perf_profiling.rst (python#103530)
  Fix unused functions warnings in instrumentation.c (pythonGH-103515)
carljm added a commit to carljm/cpython that referenced this issue Apr 14, 2023
* superopt:
  fix incompatible types
  update generated cases
  don't unnecessarily re-find args in error case
  Apply suggestions from code review
  pythongh-103532: Remove TKINTER_PROTECT_LOADTK code (pythonGH-103535)
  pythongh-103180: Add CI timeouts to all GitHub Actions jobs (python#103437)
  Remove double space in import error message (python#103458)
  ipaddress: Remove non-existent ip_str param from docstring (python#103461)
  Fix syntax typo in isolating extensions doc (python#103516)
  pythongh-103406: Modernize pos-only arguments usage in `test_signature` (python#103407)
  Proofread howto/perf_profiling.rst (python#103530)
  Fix unused functions warnings in instrumentation.c (pythonGH-103515)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

1 participant