Skip to content
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.

Sign up for GitHub

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

Attempt to fix mypy failure on ci #8128

Closed
wants to merge 1 commit into from
Closed

Attempt to fix mypy failure on ci #8128

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion torchvision/datasets/vision.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,11 @@ def __init__(
_log_api_usage_once(self)
if isinstance(root, str):
root = os.path.expanduser(root)
self.root = root
# This lines to please mypy
if root is not None:
self.root: str = root
else:
self.root = root # type: ignore[assignment]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for fixing this @vfdev-5

Would it work to just write this line unconditionally?

If there's no cleaner why to please mypy here, I'd suggest to stop type-checking this part of the code-base. It's not worth wasting time on.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if we can write it in single line...

Technically, mypy is right as root arg is Optional[str], so self.root type is Optional[str]. In subclasses we are passing root string, but self.root inherits the original type: Optional[str].

we can force the cast as (+/- some type ignores)

self.root: str = root

but this wont be correct.

Alternatively, as you say we can stop type checking this code and all related subclasses...

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just seen that this PR is on the origin of this problem: #8124


has_transforms = transforms is not None
has_separate_transform = transform is not None or target_transform is not None
Expand Down