Properly fix dataset test that passes by accident #3434
Conversation
torchvision/datasets/coco.py
Outdated
| try: | ||
| self.coco = COCO(annFile) | ||
| except FileNotFoundError as error: | ||
| raise RuntimeError(f"The file {annFile} does not exist or is corrupt.") from error |
There was a problem hiding this comment.
I'm not sure we should be changing the call-sites so that the current test infra works. Plus, FileNotFoundError is a meaningful type of error to be raised in here. Would your plan be to change all locations in the code to always raise RuntimeError?
There was a problem hiding this comment.
I'm not sure we should be changing the call-sites so that the current test infra works.
IMO every dataset should handle corrupted or non-existent files properly. I wouldn't do that to satisfy our tests, but rather because it can make the life of the user easier with a descriptive error message. Plus, this is what you agreed to
Lines 398 to 401 in 67681a7
Plus,
FileNotFoundErroris a meaningful type of error to be raised in here.
I agree. I'll update the test infrastructure.
Would your plan be to change all locations in the code to always raise
RuntimeError?
That was the original plan, yes. Maybe we can differentiate between RuntimeError for corrupted and FileNotFoundError for non-existent files. If we want to make sure we could also opt to create an FileCorruptedError(Exception) and check for that instead of a more general RuntimeError.
Codecov Report
@@ Coverage Diff @@
## master #3434 +/- ##
==========================================
+ Coverage 76.00% 76.11% +0.11%
==========================================
Files 105 105
Lines 9697 9697
Branches 1556 1556
==========================================
+ Hits 7370 7381 +11
+ Misses 1841 1836 -5
+ Partials 486 480 -6
Continue to review full report at Codecov.
|
Summary: * make UsageError an Exception rather than RuntimeError * separate fake data injection and dataset args handling * adapt tests for Coco * fix Coco implementation * add documentation * fix VideoDatasetTestCase * adapt UCF101 tests * cleanup * allow FileNotFoundError for test without fake data * Revert "fix Coco implementation" This reverts commit e2b6938. * lint * fix UCF101 tests Reviewed By: fmassa Differential Revision: D26756278 fbshipit-source-id: d2b3a7258e6dcf63595c5c4388453d2821746d79
Fixes #3427