-
Notifications
You must be signed in to change notification settings - Fork 3
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
Fix type hints #125
Fix type hints #125
Conversation
@@ -396,7 +398,7 @@ def _get_or_add_orig_datablock(self, key: Union[int, str]) -> OrigDatablock: | |||
|
|||
def make_upload_model(self) -> Union[UploadDerivedDataset, UploadRawDataset]: |
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.
Can UploadDerivedDataset
and UploadRawDataset
be TypeVar
instead?
If the model is Type[UploadRawDataset]
, the returned model is UploadRawDataset
for example, right...?
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.
But this method doesn't take any arguments except self
. So how would you match the type of the model?
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.
Oh Sorry I thought the model was also an input. Never mind...
@@ -86,7 +88,7 @@ def to_local(self) -> PurePath: | |||
|
|||
def __truediv__(self, other: Union[str, RemotePath]) -> RemotePath: | |||
"""Join two path segments.""" | |||
if isinstance(other, (PurePath, Path)): | |||
if isinstance(other, (PurePath, Path)): # type: ignore[unreachable] | |||
raise TypeError("OS paths are not supported when concatenating RemotePath.") |
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.
I have a question how a type-checker
would decide unreachable...
will this line below will also be considered unreachable...?
if not isinstance(other, (str, RemotePath)) and isinstance(other, (PurePath, Path)):
raise ...
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.
I guess so. It must be based on the argument type hints. And mypy then assumes that no invalid type is ever passed in.
No description provided.