Skip to content
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

Give all upgraders a progress bar #1670

Conversation

WilliamHPNielsen
Copy link
Contributor

All the DB upgrades should have a progress bar to make the output of the upgrading less confusing.

Changes proposed in this pull request:

  • Add a progress bar to all updaters

This PR doesn't feel particularly DRY, but I could only think of complicated ways of adding the progress bar to the upgrader decorator.

@QCoDeS/core

@codecov
Copy link

codecov bot commented Aug 15, 2019

Codecov Report

Merging #1670 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1670      +/-   ##
==========================================
+ Coverage   66.94%   66.94%   +<.01%     
==========================================
  Files         144      144              
  Lines       17804    17807       +3     
==========================================
+ Hits        11918    11921       +3     
  Misses       5886     5886

@jenshnielsen
Copy link
Collaborator

Could I suggest that we switch to using from tqdm.auto import tqdm

That gives a much nicer output in the notebook for instance.

Compare

newupgrade

to

oldupgrade

@jenshnielsen
Copy link
Collaborator

Forget that. It seems like that does not work in Spyder :(

@jenshnielsen
Copy link
Collaborator

I wish we could at least avoid it printing to std err but use regular std out

@jenshnielsen
Copy link
Collaborator

I think we can do that with tqdm(range(1, no_of_runs+1), file=sys.stdout)

Happy to push that change if you are ok with it

@WilliamHPNielsen
Copy link
Contributor Author

@jenshnielsen, I like your suggestions. Too bad with spyder. But going for stdout is already an improvement. Push away :)

@jenshnielsen
Copy link
Collaborator

@WilliamHPNielsen pushed the change

@jenshnielsen jenshnielsen merged commit 407276c into microsoft:master Aug 15, 2019
jenshnielsen added a commit that referenced this pull request Aug 19, 2019
…ll_upgrades

Give all upgraders a progress bar
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants