-
Notifications
You must be signed in to change notification settings - Fork 7.1k
feat: expose loader argument in Country211 and EuroSAT. #8922
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
Conversation
🔗 Helpful Links🧪 See artifacts and rendered test results at hud.pytorch.org/pr/pytorch/vision/8922
Note: Links to docs will display an error until the docs builds have been completed. This comment was automatically generated by Dr. CI and updates every 15 minutes. |
57f1528
to
551863e
Compare
@@ -21,6 +21,7 @@ class Country211(ImageFolder): | |||
target_transform (callable, optional): A function/transform that takes in the target and transforms it. | |||
download (bool, optional): If True, downloads the dataset from the internet and puts it into | |||
``root/country211/``. If dataset is already downloaded, it is not downloaded again. | |||
loader (callable, optional): A function to load an image given its path. |
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.
Thanks for the PR @GdoongMathew ,
Here and below, we should specify that the default is to use PIL, but that we encourage users to try to use decode_image()
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.
added. I also updated the one in ImageNet
dataset as well.
transform=transform, | ||
target_transform=target_transform, | ||
loader=loader, | ||
) |
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.
How do you feel about writing a basic test for this? Hopefully this can fit in <10 lines of code and we can re-use the same test across most datasets?
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.
Added. Hopefully, for the rest of the test classes, one could update their SUPPORT_TV_IMAGE_DECODE
attribute whenever they start to support loader
argument.
image.load() | ||
return image | ||
|
||
with unittest.mock.patch("PIL.Image.open", new=new): | ||
with unittest.mock.patch(open.__module__ + "." + open.__qualname__, new=new): |
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.
Damn, I hope we never have to change / fix this ever lol.
Thank you so much for pushing through the test @GdoongMathew
Partially fix #8762