-
Notifications
You must be signed in to change notification settings - Fork 323
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
Fix datamodules ImportError and add tests #380
Fix datamodules ImportError and add tests #380
Conversation
Codecov Report
@@ Coverage Diff @@
## master #380 +/- ##
==========================================
- Coverage 82.03% 81.10% -0.94%
==========================================
Files 100 100
Lines 5639 5725 +86
==========================================
+ Hits 4626 4643 +17
- Misses 1013 1082 +69
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
shall we add some tests that we do not break these imports in the future? |
@Borda Yes, let's. |
may we add it as part of this PR? |
@Borda Just added the import tests when optional packages are unavailable. How does it look? |
We still have similar problems in importing datamodules when dependencies are not met... e.g. |
I may be thinking about some automatic way lets say parse the |
I too will give a thought. Solution doesn't seem very straightforward to me 😅 |
I'm aware that there is inconsistency in how we check if optional packages are available (i.e. |
Co-authored-by: Jirka Borovec <Borda@users.noreply.github.com>
@Borda mind having a look again? |
What does this PR do?
Fixes #379.
Bacause raising
ModuleNotFoundError
inpl_bolts/datamodules/sklearn_datamodule.py
makesfrom pl_bolts.datamodules import SklearnDataModule
raiseImportError
as reported in the issue, this PR fixes the problem by replacing it withwarn_missing_pkg
.Before submitting
PR review
Anyone in the community is free to review the PR once the tests have passed.
If we didn't discuss your PR in Github issues there's a high chance it will not be merged.
Did you have fun?
Make sure you had fun coding 🙃