Now unpacking kwargs does not include args with default values

[sobolevn]

I am not sure this is correct. Please, see my comments:

• Don't map actual **kwargs to formal *args #6096 (comment)
• Generic function can't make function call with mapping unpacking #11583 (comment)

Closes #11583

[sobolevn]

When functions do not have a default value - we still raise a type error. Because we are not sure that **kwargs will match existing argument. But, when there's a default - we have nothing to worry about in any case.

Am I missing something?

[sobolevn]

Looks like this PR has fixed quite a lot of type: ignore stuff.

CC @Akuli: Can you please take a look at this part?

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/run/no_terminal.py:86: error: Incompatible types in assignment (expression has type "Popen[str]", variable has type "Optional[Popen[bytes]]")  [assignment]
+ porcupine/plugins/run/no_terminal.py:100: error: Item "None" of "Optional[Popen[bytes]]" has no attribute "stdout"  [union-attr]
+ porcupine/plugins/run/no_terminal.py:108: error: Item "None" of "Optional[Popen[bytes]]" has no attribute "wait"  [union-attr]
+ porcupine/plugins/git_right_click.py:49: error: "str" has no attribute "decode"; maybe "encode"?  [attr-defined]
+ porcupine/plugins/git_right_click.py:50: error: "str" has no attribute "decode"; maybe "encode"?  [attr-defined]
+ porcupine/plugins/langserver.py:783: error: Argument 1 to "LangServer" has incompatible type "Popen[str]"; expected "Popen[bytes]"  [arg-type]

[Akuli]

porcupine.utils has a dict like subprocess_kwargs: dict[str, Any]. This PR seems to make it not work at all with the complicated overloads that typeshed defines for subprocesses.

Here mypy thinks that output becomes a string, when in fact it is bytes:

        output = subprocess.check_output(
            ["git", "status", "--porcelain", "--", str(path)],
            cwd=git_cwd,  # pathlib.Path
            **utils.subprocess_kwargs,
        )

Somewhat similar problem with this, mypy thinks that process has type Popen[str] (i.e. reading from stdin, stdout or stderr returns a string) when it is in fact Popen[bytes]:

            process = subprocess.Popen(
                command,  # List[str]
                stdin=subprocess.PIPE,
                stdout=subprocess.PIPE,
                stderr=subprocess.PIPE,
                **utils.subprocess_kwargs,
            )

Here's the craziest one: mypy thinks that self._shell_process may be None, even though it obviously can't be.

    def __init__(self, ...):
        ...
        self._shell_process: subprocess.Popen[bytes] | None = None
        ...

    def _thread_target(self, ...) -> None:
        try:
            self._shell_process = subprocess.Popen(
                command,  # str
                cwd=self.cwd,  # pathlib.Path
                stdout=subprocess.PIPE,
                stderr=subprocess.STDOUT,
                shell=True,
                env=env,  # dict[str, str]
                **utils.subprocess_kwargs,
            )
        except OSError as e:
            ...
            return

        assert self._shell_process.stdout is not None

[github-actions]

Diff from mypy_primer, showing the effect of this PR on open source code:

streamlit (https://github.com/streamlit/streamlit.git)
+ lib/streamlit/server/server.py:437: error: Unused "type: ignore" comment

core (https://github.com/home-assistant/core.git)
+ homeassistant/runner.py:94: error: Unused "type: ignore" comment
+ homeassistant/runner.py:98: error: Unused "type: ignore" comment

pydantic (https://github.com/samuelcolvin/pydantic.git)
+ pydantic/datetime_parse.py:170: error: Unused "type: ignore" comment

rotki (https://github.com/rotki/rotki)
+ rotkehlchen/tests/utils/database.py:71: error: Unused "type: ignore" comment

porcupine (https://github.com/Akuli/porcupine.git)
+ porcupine/plugins/run/no_terminal.py:86: error: Incompatible types in assignment (expression has type "Popen[str]", variable has type "Optional[Popen[bytes]]")  [assignment]
+ porcupine/plugins/run/no_terminal.py:100: error: Item "None" of "Optional[Popen[bytes]]" has no attribute "stdout"  [union-attr]
+ porcupine/plugins/run/no_terminal.py:108: error: Item "None" of "Optional[Popen[bytes]]" has no attribute "wait"  [union-attr]
+ porcupine/plugins/git_right_click.py:49: error: "str" has no attribute "decode"; maybe "encode"?  [attr-defined]
+ porcupine/plugins/git_right_click.py:50: error: "str" has no attribute "decode"; maybe "encode"?  [attr-defined]
+ porcupine/plugins/langserver.py:783: error: Argument 1 to "LangServer" has incompatible type "Popen[str]"; expected "Popen[bytes]"  [arg-type]

pandas (https://github.com/pandas-dev/pandas.git)
+ pandas/core/generic.py:4408: error: Unused "type: ignore[arg-type]" comment
+ pandas/core/generic.py:4472: error: Unused "type: ignore[arg-type]" comment

sphinx (https://github.com/sphinx-doc/sphinx.git)
+ sphinx/ext/mathjax.py:87: error: Unused "type: ignore" comment

paasta (https://github.com/yelp/paasta.git)
+ paasta_tools/utils.py:2660: error: Argument 1 to "write" of "IO" has incompatible type "bytes"; expected "str"
+ paasta_tools/utils.py:2674: error: "str" has no attribute "decode"; maybe "encode"?

pyppeteer (https://github.com/pyppeteer/pyppeteer.git)
- pyppeteer/launcher.py:148: error: Argument 2 to "Popen" has incompatible type "**Dict[str, Optional[Any]]"; expected "int"
- pyppeteer/launcher.py:148: error: Argument 2 to "Popen" has incompatible type "**Dict[str, Optional[Any]]"; expected "bool"
- pyppeteer/launcher.py:148: error: Argument 2 to "Popen" has incompatible type "**Dict[str, Optional[Any]]"; expected "str"

boostedblob (https://github.com/hauntsaninja/boostedblob.git)
+ boostedblob/copying.py:319: error: Unused "type: ignore" comment

[JukkaL]

I need to think about this carefully, but my initial feeling is that this is reasonable. Type checking **kwargs strictly is kind of impractical, so accepting calls that might be fine under somewhat optimistic assumptions seems consistent with other things we do, such as accepting tuple unpacking with variable-length lists on the RHS.

However, some users might prefer the old behavior, and it could be nice to add a strictness option that implements strict **kwargs checking (and possibly strict checking of tuple unpacking). This is just a random idea and no need to do anything about it.

[Akuli]

Should the existence of **kwargs in a function call affect which overload is chosen?

[sobolevn]

This is just a random idea and no need to do anything about it.

I will create a new issue out of it afterwards! Thanks!

Should the existence of **kwargs in a function call affect which overload is chosen?

Intersting question. I will try out several overloads.

[ArgentFalcon]

I wanted to see if there was any action on this PR, as we are also running into this issue.