Update asyncio subprocess optional **kwargs.

[socketpair]

Made strongly according to documentation (possible Sequence vs Iterable vs Collection bug).

But did not check.

[socketpair]

@JelleZijlstra

[JelleZijlstra]

These kwargs are delegated to subprocess.Popen, so it would be good to match the types exactly. It seems that 3.11 added another new kwarg, process_group: int | None = ..., so we should add a 3.11 branch to these stubs too.

pass_fds is Any in stdlib/subprocess.pyi, but from looking at the code Iterable[int] does appear to be the right type. Feel free to fix that type in this PR too.

[socketpair]

@JelleZijlstra done

[socketpair]

subprocess.py:

                    fds_to_keep = set(pass_fds)
                    fds_to_keep.add(errpipe_write)
                    self.pid = _posixsubprocess.fork_exec(
                            args, executable_list,
                            close_fds, tuple(sorted(map(int, fds_to_keep))),

Documentation: https://docs.python.org/3/library/subprocess.html

pass_fds is an optional sequence of file descriptors to keep open between the parent and child.

Yes, it's allowed to pass non-int. But I think it's a bug to pass non-int.

pypeeter code:

        options = dict()
        options['env'] = self.env
        if not self.dumpio:
            # discard stdout, it's never read in any case.
            options['stdout'] = subprocess.DEVNULL
            options['stderr'] = subprocess.STDOUT

        self.proc = subprocess.Popen(  # type: ignore
            self.cmd, **options, )

[socketpair]

Since bool(some _generator) is always True, and code contains if pass_fds then it should be Collection.

[JelleZijlstra]

The new error seems correct, though maybe mypy should listen to the type ignore: https://github.com/pyppeteer/pyppeteer/blob/7245ee0bccd97d43e45b801319dd6cbae860292a/pyppeteer/launcher.py#L148

[JelleZijlstra]

subprocess has this as just int | None, which looks correct based on the C code.

[socketpair]

I dont' understand what you wanted to say.

[JelleZijlstra]

In subprocess.pyi, the type of the process_group parameter is int | None, not str | int | None. Based on my reading of the relevant C code, int | None appears to be correct.

[AlexWaygood]

@socketpair, you haven't addressed this comment here. Jelle is asking you to change the annotation for the process_group parameter (both for this function and all other functions that have this parameter) to be int | None instead of str | int | None. You need to either make the change or explain why you think that this would be incorrect.

[socketpair]

Really. it's my bug. Will fix in next commit.

[socketpair]

What should I do to move this PR to merging ?

[socketpair]

@JelleZijlstra done

[hauntsaninja]

I'm not seeing the change, did you forget to push?

[socketpair]

@JelleZijlstra sorry for calling again. What should I change (what is wrong) in current PR? Please mark these changes somehow in order to distinguish from just speaking. Now, I will just rebase, no other changes.

[AlexWaygood]

Requesting changes until #9177 (comment) is addressed

[socketpair]

I don't understand new Github review semantics. I just changed what you requested. I don't know if I need to tap something in GH

Requested changes were made

[github-actions]

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

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