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

Add executable kwarg to trio.run_process type hints #3183

Open
dj-foxxy opened this issue Jan 12, 2025 · 6 comments
Open

Add executable kwarg to trio.run_process type hints #3183

dj-foxxy opened this issue Jan 12, 2025 · 6 comments
Labels
good first issue typing Adding static types to trio's interface

Comments

@dj-foxxy
Copy link

The executable kwarg is missing from the type hints of trio.run_process. It works at run time but Pyright is complaining.

@A5rocks
Copy link
Contributor

A5rocks commented Jan 12, 2025

The executable kwarg is missing from the type hints of trio.run_process. It works at run time but Pyright is complaining.

Absolutely, and it looks like we're missing other kwargs in the signature too. (and same for trio.open_process) Namely:

  • executable (of course)
  • group (3.9+, unix)
  • extra_groups (3.9+, unix)
  • user (3.9+, unix)
  • umask (3.9+, unix)
  • pipesize (3.10+, linux but is ignored so all platforms? or maybe unix)
  • process_group (3.11+, unix)

Feel free to make a PR -- you'll have to add some if sys.version_info checks and update the cheat sheet comment on top of the overloads.


If you no longer want Latta AI to attempt solving issues on your repository, you can block this account.

Done!

@A5rocks A5rocks added good first issue typing Adding static types to trio's interface labels Jan 12, 2025
@jakkdl
Copy link
Member

jakkdl commented Jan 12, 2025

That if TYPE_CHECKING block is gonna become an absolute monstrosity - it's already a ~third of the file with 400 lines. This might warrant a separate file (_subprocess_type_hints.py?) and maybe also a dedicated test that compares the type-checker inferred signature with that of subprocess.run (although given the false alarm rate on test_static_tool_sees_[...] I'm not terribly excited about that)

@A5rocks
Copy link
Contributor

A5rocks commented Jan 12, 2025

maybe also a dedicated test that compares the type-checker inferred signature with that of subprocess.run (although given the false alarm rate on test_static_tool_sees_[...] I'm not terribly excited about that)

I don't think this is possible because typeshed structures its overloads differently to ours (eg no platform specific ones)

@dj-foxxy
Copy link
Author

Happy to give this a go. Can I create script than generates it? I feel like doing this by hand is too much work and maintenance nightmare when they add more kwargs.

@A5rocks
Copy link
Contributor

A5rocks commented Jan 13, 2025

Can I create script than generates it? I feel like doing this by hand is too much work and maintenance nightmare when they add more kwargs.

If you want to! I don't think it's necessary: we'll just change the 2 overloads for things on Unix into 6 overloads across 3 different branches. But also 6 overloads still is somewhat large, plus open_process. (If you do make a script, don't bother with making it work completely on its own IMO)

@jakkdl
Copy link
Member

jakkdl commented Jan 13, 2025

I think maintaining a script across platforms etc is going to be way more work than adding a kwarg at most once a year with a new python release. Lotsa stuff needs updating with new releases anyway.
But for generating the initial set, an LLM and/or a quick script might well be worth it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue typing Adding static types to trio's interface
Projects
None yet
Development

No branches or pull requests

4 participants
@dj-foxxy @jakkdl @A5rocks and others