-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Exact return types instead of shutil._PathReturn #13767
Exact return types instead of shutil._PathReturn #13767
Conversation
Diff from mypy_primer, showing the effect of this PR on open source code: prefect (https://github.com/PrefectHQ/prefect)
+ src/prefect/filesystems.py:162: error: Value of type variable "_StrPathT" of "copytree" cannot be "Optional[str]" [type-var]
- src/prefect/filesystems.py:162: error: Argument 2 to "copytree" has incompatible type "Optional[str]"; expected "Union[str, PathLike[str]]" [arg-type]
mkosi (https://github.com/systemd/mkosi)
+ mkosi/__init__.py:3307:16: error: Incompatible return value type (got "tuple[Union[Path, str], None, None, list[Path]]", expected "tuple[Optional[Path], Optional[str], Optional[Path], list[Path]]") [return-value]
+ mkosi/__init__.py:3320:12: error: Incompatible types in assignment (expression has type "Union[Path, str]", variable has type "Path") [assignment]
+ mkosi/__init__.py:3329:12: error: Incompatible types in assignment (expression has type "Union[Path, str]", variable has type "Path") [assignment]
|
Obligatory link to python/typing#566 ( |
Would love to see an official proposal for that one. I don't think I have the time/motivation/expertise to tackle that though. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Considering the low primer impact and the fact that this is a genuine trap (and a bug in my opinion) this change seems good to me.
python/typeshed#13767 narrows the typing for shutil.copy2() so let's address that by fixing our typing.
python/typeshed#13767 narrows the typing for shutil.copy2() so let's address that by fixing our typing.
Just for reference: This broke us downstream in mkosi. This was fixed with systemd/mkosi#3675. For us the change was rather small, but other people might be broken more significantly (or maybe not looking at your mypy_primer bot) and some more dependant typing that keeps the type of what is passed through would be nice—though I don't know if shutil guarantees this. Regarding your mypy_primer action: If you see us affected by a change, feel free to reach out, we have a largeish stdlib-only fully-typed codebase that's running current mypy and pyright pretty much daily. We are a bit limited by our current minimum required Python (3.9) for the moment, but we strive to be as fully typed with as few casts, coercions or ignores as possible. |
@behrmann I'm sorry for the breakage. I'll try to keep in mind to contact mkosi when we notice primer breakage. Unfortunately, this area of This should probably fixed in CPython. I've opened python/cpython#132322 for now to track this at least. |
Closes #13508
#13508 (comment)
I'm not hard set on this, it's a balance between type safety, and a (imo) possibly very small annoyance in the specific case you know for certain that you're moving/copying a file + are passing in a
PathLike
+ need the result to not be a string.But I'm at least demonstrating the suggestion, and can dial it back to an
Any
return type.