-
Notifications
You must be signed in to change notification settings - Fork 7k
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
Remove model download from tests #2806
Conversation
@fmassa I understand the issue to fix and this can help. But how about test_hub.py and test_onnx.py which also download pretrained wieghts ? Line 32 in 02f5c1f
Line 379 in 02f5c1f
What do you think to accept failing if we catch |
@vfdev-5 ideally they should all avoid downloading weights, and we should fix the tests for this. I've messed up with both of the tests here, I'll be fixing them in a few moments |
Codecov Report
@@ Coverage Diff @@
## master #2806 +/- ##
==========================================
- Coverage 73.31% 73.17% -0.14%
==========================================
Files 99 99
Lines 8724 8728 +4
Branches 1373 1373
==========================================
- Hits 6396 6387 -9
- Misses 1909 1925 +16
+ Partials 419 416 -3
Continue to review full report at Codecov.
|
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.
@fmassa LGTM, thanks for the PR.
I've left only 1 comment to confirm that a corner-case is handled as intended. If it is, feel free to merge.
* Remove model download from tests * Refactor trainable_layers checks in detection models * Bugfix * Finish tests and fixes * Fix lint
* Remove model download from tests * Refactor trainable_layers checks in detection models * Bugfix * Finish tests and fixes * Fix lint
They can introduce flakiness due to connection errors, and slow tests down, while not being needed for the tests that we currently have.