Add type annotations to pip._internal.utils.filesystem #5769
Conversation
There was a problem hiding this comment.
I'm not an expert in type annotations (I've barely used them) but I'd hope that whatever annotations we do add should be understandable by non-experts, so hopefully that doesn't disqualify me from commenting :-)
I'd rather we just annotated function signatures, and didn't try to sprinkle annotations through the body of the function. Certainly there seem to be some annotations here that don't really seem to add anything useful.
|
|
||
| def check_path_owner(path): | ||
| # type: (str) -> List[str] |
There was a problem hiding this comment.
Isn't this wrong? It says we're returning a List[str] but we actually return a boolean...
There was a problem hiding this comment.
Yep. It is. This should be (str) -> bool and the List import is redundant once this change is made.
| except OSError: | ||
| return False | ||
| return path_uid == 0 | ||
| return path_uid == 0 # type: int |
There was a problem hiding this comment.
This seems like an odd annotation. What's being annotated? The expression? (Which is obviously boolean).
Yep. IMO the point of this exercise is those who read/use the code should get a quick summary of what types to expect from the type annotation. |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
lock
bot
added
the
auto-locked
Links to #4748, I just added types to a small file.
Once this passed, I'm going to add more in next PRs, thanks!