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
Add a test for progress bar #2150
Conversation
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.
Thanks for sorting this out.
All the calls to construct progress bar should move to where .start()
was originally. I would also prefer if the tests where a bit more explicit about what they were testing. Having tests decide what to test based on the code is a bad idea, because the code can easily be wrong (e.g. we might forget to add something to the list of progress bars) and it's good to have a pattern of "if the code is changed, the tests should change too".
qutip/tests/test_progressbar.py
Outdated
bar = progress_bars[pbar](N) | ||
except ImportError: | ||
# ipython or tqdm could be missing | ||
pytest.skip(reason="module not available") |
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.
We should probably list the progress bars explicitly and mark which ones to skip. Otherwise this will skip tests if something fails to import and it might take a long time to realize.
Description
start
private.start
was mostly used right after the initialization. The docstring indicated that it was not needed, but it was for most bars. EnhancedTextProgressBar not coherent with docstring of BaseProgressBar #2148HTMLProgressBar
import ipython.Related issues or PRs
Fix the issue #2148 for master.
Replace #2127