-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Checking ipywidgets is installed for ensure tqdm working #2417
Checking ipywidgets is installed for ensure tqdm working #2417
Conversation
Just searched for |
…pywidgets is installed.
647126a
to
5b19bb3
Compare
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.
Nice!
from tqdm.auto import tqdm | ||
except ImportError as e: |
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.
could we use
import importlib
if importlib.util.find_spec("ipywidgets") is not None:
import ipywidgets
from tqdm.auto import tqdm
else:
from tqdm import tqdm
? (brain compiled, may have typos)
ImportError is not suitable here in general.
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.
should probably also add "pragma no cover " not to decrease coverage
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.
Yes, sure. Thanks!
Codecov Report
@@ Coverage Diff @@
## master #2417 +/- ##
======================================
- Coverage 89% 89% -0%
======================================
Files 69 69
Lines 5476 5482 +6
======================================
+ Hits 4856 4860 +4
- Misses 620 622 +2 |
This pull request is now in conflict... :( |
@olineumann love the PR! this is a great addition |
What does this PR do?
Some users having problems with the progress bar in combination with jupyter notebook/lab. Sometimes it crashes because of missing dependencies, sometimes the progress bar isn't displayed correctly. Normally tqdm should select the correct package via tqdm.auto which is used in the code, but as some users reporting (not only in pytorch_lightning) it seems not working correctly.
Since pytorch_lightning doesn't want to add ipywidgets (which often is the problem) to the dependencies it would be better to check if this package is installed before importing tqdm.auto and import only tqdm of not.
As an example run this in jupyter without ipywidgets installed:
Fixes #1687
Before submitting
Not needed I think?
Tests passed locally.
Not necessary.
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 🙃