fix: Update sys.exit and SystemExit exception to have the same types - #8554
AlexWaygood merged 4 commits into
Conversation
Sorry, something went wrong.
This comment has been minimized.
AlexWaygood
left a comment
There was a problem hiding this comment.
Thanks, this looks good. The mypy_primer hits are a little unfortunate, but they're arguably "true positives", and I can live with them.
Two things that might be good to change, before merging:
- Presumably builtins.quit() and builtins.exit() should be changed in the same way as sys.exit()?
- Can we define a type alias in the sys module for str | int | None, and then use that for all of these functions, and the code attribute of SystemExit? I think it would help ensure that they don't drift out of sync in the future.
Sorry, something went wrong.
|
Added 🙂 |
Sorry, something went wrong.
This comment has been minimized.
AlexWaygood
left a comment
There was a problem hiding this comment.
LGTM, but I'll wait a little while before merging to see if any other maintainers have any thoughts :)
Sorry, something went wrong.
This comment has been minimized.
|
works for me, thanks for the review |
Sorry, something went wrong.
|
Diff from mypy_primer, showing the effect of this PR on open source code: cwltool (https://github.com/common-workflow-language/cwltool)
+ cwltool/main.py:1322:24: error: Incompatible return value type (got "Union[str, int, None]", expected "int") [return-value]
+ tests/util.py: note: In function "get_main_output":
+ tests/util.py:110:14: error: Incompatible types in assignment (expression has type "Union[str, int, None]", variable has type "int") [assignment]
sphinx (https://github.com/sphinx-doc/sphinx)
+ sphinx/cmd/quickstart.py: note: In function "main":
+ sphinx/cmd/quickstart.py:551:16: error: Incompatible return value type (got "Union[str, int]", expected "int") [return-value]
mypy (https://github.com/python/mypy)
+ mypy/api.py:62: error: Incompatible types in assignment (expression has type "Union[str, int, None]", variable has type "int") [assignment]
|
Sorry, something went wrong.
fix: #8513
This updates the types of sys.exit and SystemExit with the recommendations of @Akuli. The issue had received 👍 s from three team members, so I wanted to help move this along for the team.