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

os.waitid can return None #10564

Closed
gotyaoi opened this issue Aug 12, 2023 · 3 comments · Fixed by #10589
Closed

os.waitid can return None #10564

gotyaoi opened this issue Aug 12, 2023 · 3 comments · Fixed by #10589

Comments

@gotyaoi
Copy link
Contributor

gotyaoi commented Aug 12, 2023

Specifically, it can return None when its options argument contains os.WNOHANG.

If WNOHANG is specified and there are no matching children in the requested state, None is returned.

I'm not sure there's any way to express that with overrides, given that it depends on the value of a specific argument. I'm also not sure of the ergonomic effect of making it return os.waitid_result | None.

@Akuli
Copy link
Collaborator

Akuli commented Aug 12, 2023

Since there are 5 flags that can be bitwise-ORed, there are 16 combinations of flags where WNOHANG is set and 16 more where it isn't set. We could add overloads with unions that have 16 members, or just return waitid_result | Any, similarly to how regexes work (see e.g. #10526).

I would prefer the simpler waitid_result | Any solution, because os.waitid() is not exactly the most commonly used function in the stdlib. Usually you let subprocess.run() or similar do the waiting or you.

@JelleZijlstra
Copy link
Member

If it's rarely used, maybe it's better to go the more type-safe route and use waitid_result | None as the annotation, forcing users to think about the possibility that it may return None.

@gotyaoi
Copy link
Contributor Author

gotyaoi commented Aug 15, 2023

While part of me wants it to infer the return type, a more realistic part of me thinks that being forced to check for None is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants