Skip to content

Conversation

@pmeier
Copy link
Contributor

@pmeier pmeier commented Nov 23, 2023

Supersedes #8128. Error caused by #8124.

@pytorch-bot
Copy link

pytorch-bot bot commented Nov 23, 2023

🔗 Helpful Links

🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8134

Note: Links to docs will display an error until the docs builds have been completed.

✅ You can merge normally! (3 Unrelated Failures)

As of commit 9ae3188 with merge base 83090c2 (image):

BROKEN TRUNK - The following jobs failed but were present on the merge base:

👉 Rebase onto the `viable/strict` branch to avoid these failures

This comment was automatically generated by Dr. CI and updates every 15 minutes.

random_offset: int = 0,
) -> None:
super().__init__(None, transform=transform, target_transform=target_transform) # type: ignore[arg-type]
super().__init__(transform=transform, target_transform=target_transform)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Driveby, since we no longer need to pass None after #8124.

Copy link
Member

@NicolasHug NicolasHug left a comment

Choose a reason for hiding this comment

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

Thanks Philip. One Q but LGTM anyway

def __init__(
self,
root: Optional[str] = None,
root: str = None, # 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.

can we still keep the Optional[str] type which is technically correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can't. If we use Optional[str] as annotation, mypy correctly infers that for the self.root attribute. And with that we are back at the issues we are currently seeing, i.e. all child datasets that perform anything with self.root would now need to check that self.root is in fact a str and not None.

@pmeier pmeier merged commit 3eb4333 into pytorch:main Nov 23, 2023
@pmeier pmeier deleted the fix-mypy branch November 23, 2023 20:31
facebook-github-bot pushed a commit that referenced this pull request Jan 15, 2024
Reviewed By: vmoens Differential Revision: D52539016 fbshipit-source-id: d1726730d1d432b32f1ab36699ec5e361be75652
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment