Add a generic test for the datasets #1015
Conversation
There was a problem hiding this comment.
This looks good, thanks a lot!
I have a few comments, in particular around the fact that generic_dataset is not generic: many of our datasets do not return (Image, int), so I'd rather not give the impression that they should. Renaming the function (and maybe moving it into Tester) would be better I think.
test/test_datasets.py
Outdated
| @@ -9,6 +9,14 @@ | |||
| from fakedata_generation import mnist_root, cifar_root, imagenet_root | |||
|
|
|||
|
|
|||
| def generic_dataset_test(tester, dataset, num_images=1, cls='fakedata'): | |||
There was a problem hiding this comment.
Can you maybe rename this function to basic_classification_dataset_test or something that includes classification in it? Not all datasets have the class_to_idx attribute in it, so I'd not want (at least for now) to force people to add it just to make it compliant with this test. Maybe in the future we could have a ClassificationDataset that has this attribute, and then we could start enforcing the presence of this attribute.
Also, any particular reason why this is not a method in Tester? If it doesn't start with test, it won't be run by the Tester, so we can have helper functions inside Tester.
There was a problem hiding this comment.
[...] many of our datasets do not return
(Image, int), so I'd rather not give the impression that they should.
Shame on me, I keep forgetting that.
Also, any particular reason why this is not a method in
Tester? If it doesn't start withtest, it won't be run by theTester[...]
I did not know that. Thanks for the clarification.
- renamed generic*() to generic_classification*() - moved function inside Tester - test class_to_idx attribute outside of generic_classification*()
Codecov Report
@@ Coverage Diff @@
## master #1015 +/- ##
==========================================
+ Coverage 63.33% 63.37% +0.04%
==========================================
Files 65 65
Lines 5149 5152 +3
Branches 772 772
==========================================
+ Hits 3261 3265 +4
+ Misses 1665 1664 -1
Partials 223 223
Continue to review full report at Codecov.
|
This adds a generic test, which should be mandatory for all datasets. It tests
PIL.Imageandintwhen indexed, andTo make this generic test applicable to the
*MNISTdatasets, I removed the randomness in the label generation.