Closed AlexWaygood closed 1 year ago
Shouldn't we fix the parameter name in the stub instead in this case?
⚠ Diff showing the effect of this PR on how stubdefaulter would alter typeshed's stdlib:
Shouldn't we fix the parameter name in the stub instead in this case?
We should do that as well, yes.
The underlying issue here is that stubtest doesn't precisely verify parameter names for positional-only parameters -- it uses a heuristic instead: https://github.com/python/mypy/blob/c03e979ca06c3bf082a4cd07458a1bc3205dc5e5/mypy/stubtest.py#L571-L574. But that doesn't feel like the responsibility of stubdefaulter?
I don't feel too strongly though, we can close this if you like :) If you look at the primer diff, the patch was pretty good at exposing that we for some reason have quite a few __get__
methods with incorrect parameter names in typeshed, so it's been useful in any case!
I guess we could possibly have this patch autofix the parameter names as well...
I guess stubtest works that way because pos-only parameter names are often inconsistent across versions (e.g., when AC is applied to a function) or across the C and Python versions of a function. For those cases, it's reasonable for stubdefaulter to also pick up the default.
Could you limit this patch to only positional-only parameters? That feels safer.
⚠ Diff showing the effect of this PR on how stubdefaulter would alter typeshed's stdlib:
⚠ Diff showing the effect of this PR on how stubdefaulter would alter typeshed's stdlib:
⚠ Diff showing the effect of this PR on how stubdefaulter would alter typeshed's stdlib:
This change has no effect on how stubdefaulter would alter typeshed's stdlib. 🤖🎉
This change has no effect on how stubdefaulter would alter typeshed's stdlib. 🤖🎉
If I run this patch on https://github.com/python/typeshed/commit/74112dc18919084093d7aed3f26456ccf4598cf8, it produces this diff:
If the number of parameters in the stub matches the number of parameters at runtime, and there are no variadic parameters at runtime, it should be safe to add defaults to the stub even if the stub has an incorrect name for the parameter