Expose VisionDataSet class so it can be modified #876
Conversation
Codecov Report
@@ Coverage Diff @@
## master #876 +/- ##
==========================================
+ Coverage 55.17% 55.39% +0.22%
==========================================
Files 36 36
Lines 3375 3376 +1
Branches 553 553
==========================================
+ Hits 1862 1870 +8
+ Misses 1375 1370 -5
+ Partials 138 136 -2
Continue to review full report at Codecov.
|
|
@bbowles-personal I'm curious: What did you need to modify? Is this something we could also use? On what part was |
| @@ -17,6 +17,7 @@ | |||
| from .caltech import Caltech101, Caltech256 | |||
| from .celeba import CelebA | |||
| from .sbd import SBDataset | |||
| from .vision import VisionDataset | |||
|
|
|||
| __all__ = ('LSUN', 'LSUNClass', | |||
There was a problem hiding this comment.
Shouldn't VisionDataset contained in __all__ if we want to expose it?
There was a problem hiding this comment.
If we want it to be imported via from torchvision.datasets import *, then yes.
There was a problem hiding this comment.
Let me rephrase: Do we want that? An all or nothing approach seems reasonable to me.
There was a problem hiding this comment.
I'm fine with either, but for consistency let's add it there.
There was a problem hiding this comment.
In my case, I wanted to restrict the folders that are found by mucking with _find_classes but the easiest way to do that was just to inherit from VisionDataset and write my own DatasetFolder class.
If you want, I can write a pull to allow a keyword arg to ImageFolder to only allow it to find certain folders, which is actually what I need....
not really using this code for what it was intended to do so not sure if you really want that.
thanks for approving
There was a problem hiding this comment.
I can merge this as is, but I'd accept a PR adding VisionDataset to __all__
This is really helpful class, but I had a use case where I needed to modify slightly.