python / mypy

Optional static typing for Python
https://www.mypy-lang.org/
Other
18.22k stars 2.78k forks source link

stubtest: more fine-grained allowlist #13703

Open twoertwein opened 2 years ago

twoertwein commented 2 years ago

Feature

A function such as pandas.read_csv has many arguments - there are many ways in which a stub file can be incompatible with the implementation :) As far as I know, stubtest's allowlist can either ignore the entire function/method but not individual arguments or individual error codes.

Pitch The allowlist could support which argument names should be ignored. Further, it would be great if stubtest defines error codes/categories to ignore only those categories for a function/argument

pandas.read_csv.names pandas.read_csv # type: ignore[implementation-is-not-keyword-only]

AlexWaygood commented 2 years ago

Further, it would be great if stubtest defines error codes/categories to ignore only those categories for a function/argument

Yup:

https://github.com/python/mypy/blob/0a720edb1489af6da63f0731cbc66263598a5a5d/mypy/stubtest.py#L108

hauntsaninja commented 2 years ago

@twoertwein While stubtest should do something like this, it might be a little finnicky to get right. I assume your use case for this is to ignore errors from deprecate_nonkeyword_arguments? In which case I have a proposal for you: make deprecate_nonkeyword_arguments alter the function signature.

The following patch should work and passes all the tests in pandas/tests/util/test_deprecate_nonkeyword_arguments.py. If deprecate_nonkeyword_arguments is in fact most of your desire for this, and the patch looks plausible to you, let me know and I'll open a PR. Should hopefully be easier than allowlisting with error codes.

``` diff --git a/pandas/util/_decorators.py b/pandas/util/_decorators.py index f8359edaa8..f735af6724 100644 --- a/pandas/util/_decorators.py +++ b/pandas/util/_decorators.py @@ -290,14 +290,26 @@ def deprecate_nonkeyword_arguments( """ def decorate(func): + old_sig = inspect.signature(func) + if allowed_args is not None: allow_args = allowed_args else: - spec = inspect.getfullargspec(func) + allow_args = [ + p.name + for p in old_sig.parameters.values() + if p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD) + and p.default is p.empty + ] - # We must have some defaults if we are deprecating default-less - assert spec.defaults is not None # for mypy - allow_args = spec.args[: -len(spec.defaults)] + new_params = [ + p.replace(kind=p.KEYWORD_ONLY) + if (p.kind in (p.POSITIONAL_ONLY, p.POSITIONAL_OR_KEYWORD) and p.name not in allow_args) + else p + for p in old_sig.parameters.values() + ] + new_params.sort(key=lambda p: p.kind) + new_sig = old_sig.replace(parameters=new_params) num_allow_args = len(allow_args) msg = ( @@ -307,15 +319,15 @@ def deprecate_nonkeyword_arguments( @wraps(func) def wrapper(*args, **kwargs): - arguments = _format_argument_list(allow_args) if len(args) > num_allow_args: warnings.warn( - msg.format(arguments=arguments), + msg.format(arguments=_format_argument_list(allow_args)), FutureWarning, stacklevel=find_stack_level(), ) return func(*args, **kwargs) + wrapper.__signature__ = new_sig return wrapper return decorate ```
twoertwein commented 2 years ago

I assume your use case for this is to ignore errors from deprecate_nonkeyword_arguments? In which case I have a proposal for you: make deprecate_nonkeyword_arguments alter the function signature.

Wow, that would cover many cases! Are there other tools except for stubtest that benefit from exposing the "correct"/intended function signature?

hauntsaninja commented 2 years ago

I'm sure there are several, but the only one I'm aware of that I use regularly of is the ?? shortcut in IPython to show function signatures. Sounds like I should make a PR :-)

AlexWaygood commented 2 years ago

Are there other tools except for stubtest that benefit from exposing the "correct"/intended function signature?

It will also affect the signature displayed by help() in the builtin Python REPL, as well as the IPython REPL :)

twoertwein commented 1 year ago

A not thought-through idea: Would it make sense for stubtest to operate in two steps: 1) Generate pyi files based on runtime inspection and 2) then "somehow" re-use mypy/pyright to compare those new pyi files with existing pyi files.

A probably very ugly way to define "somehow": For each class/function present in both pyi files, create a new python files with:

from manual_stubs import Class as Class_manual
from stubtest_stubs import Class as Class_stubtest

class Class(Class_manual, Class_stubtest): ...

from manual_stubs import fun as fun_manual
from stubtest_stubs import fun as fun_stubtest

def fun_a(fun: <signature from fun_manual>): ...
def fun_b(fun: <signature from fun_stubtest>): ...
fun_a(fun_stubtest)
fun_b(fun_manual)

then run mypy (or any other type checkers) on this file.