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

RF: Replaces our own custom progressbar with a tqdm progressbar. #2227

Merged
merged 6 commits into from
Oct 14, 2020

Conversation

arokem
Copy link
Contributor

@arokem arokem commented Sep 24, 2020

Closes #2219.

@codecov
Copy link

codecov bot commented Sep 25, 2020

Codecov Report

Merging #2227 into master will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2227      +/-   ##
==========================================
- Coverage   91.36%   91.35%   -0.01%     
==========================================
  Files         252      252              
  Lines       32941    32932       -9     
  Branches     3465     3466       +1     
==========================================
- Hits        30095    30086       -9     
- Misses       2085     2086       +1     
+ Partials      761      760       -1     
Impacted Files Coverage Δ
dipy/data/fetcher.py 38.06% <100.00%> (-1.73%) ⬇️
dipy/info.py 100.00% <100.00%> (ø)
dipy/viz/app.py 77.47% <0.00%> (+0.49%) ⬆️

@skoudoro skoudoro added this to the 1.3 milestone Sep 25, 2020
@arokem
Copy link
Contributor Author

arokem commented Sep 28, 2020

I'm at a bit of a loss here. Why is Travis not picking up the tqdm dependency from here on the Travis DEPEND="" build?

@skoudoro
Copy link
Member

skoudoro commented Sep 28, 2020

I do not know the impact of this line but it might be because of that. I realize that we forgot it for packaging module too. However, packaging is always installed via Setuptools. I suppose this is why I had no error with this module.

@skoudoro
Copy link
Member

skoudoro commented Oct 2, 2020

Hi @arokem,

This PR is close to being done, Can you try the proposition above?

Also, specifies a minimal version.
@arokem
Copy link
Contributor Author

arokem commented Oct 14, 2020

Thanks for the tip @skoudoro! Let's see if this does the trick.

@arokem
Copy link
Contributor Author

arokem commented Oct 14, 2020

Looks like the failures are now all unrelated to this PR.

@skoudoro
Copy link
Member

Indeed, Looks good to me so merging! thanks @arokem

@skoudoro skoudoro merged commit 4bbaf69 into dipy:master Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Replace fetcher progress bar with tqdm
2 participants