Add stub for filecmp to 2and3 #955
Conversation
stdlib/2and3/filecmp.pyi
Outdated
| import sys | ||
| from typing import AnyStr, Callable, Dict, Generic, Iterable, List, Optional, Sequence, Tuple | ||
|
|
||
| _SupportsBool = object |
There was a problem hiding this comment.
Seems fine to just use bool as the type for the shallow argument to cmp. From looking at the code it does technically support any object that implements __bool__ (which is all objects), but many existing stubs use bool for arguments that don't strictly need to be bools.
There was a problem hiding this comment.
I did this because the default value is 1 in python2 and True on python3 I didn't want people who used 0 to suddenly fail. If that's not a problem I can switch it to bool or I can make it Union[int, bool] if you think it's important.
There was a problem hiding this comment.
That makes sense, I shouldn't have checked just the Python 3 code. :)
bool is a subtype of int, so maybe we could just use int, but I don't feel strongly about this.
There was a problem hiding this comment.
I made it a Union[int, bool] since that's how people should probably call it with.
stdlib/2and3/filecmp.pyi
Outdated
|
|
||
| DEFAULT_IGNORES = ... # type: List[str] | ||
|
|
||
| def cmp(f1: AnyStr, f2: AnyStr, shallow: _SupportsBool = ...) -> bool: ... |
There was a problem hiding this comment.
I'm not sure AnyStr is correct here. Jukka has some draft guidance in #871, which says that AnyStr should be used only when there are multiple arguments that must be of the same type. I don't think that's the case here.
There was a problem hiding this comment.
I believe I tested this manually and found that mixing str and bytes was bad in python3. I'll test again.
There was a problem hiding this comment.
Actually it's fine in python3 to mix bytes and str so I changed it to Union[bytes, Text]
stdlib/2and3/filecmp.pyi
Outdated
| def phase3(self) -> None: ... | ||
| def phase4(self) -> None: ... | ||
| def phase4_closure(self) -> None: ... | ||
| def report(self) -> None: ... |
There was a problem hiding this comment.
These three are documented, so I don't think they belong in the "These should probably be private" section.
There was a problem hiding this comment.
I'll just remove the comment. It's not for me to comment on how the stdlib is implemented. :)
No description provided.