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: Decrease Nosetest warning #1601

Merged
merged 2 commits into from Jul 28, 2018

Conversation

Projects
None yet
5 participants
@skoudoro
Copy link
Member

skoudoro commented Jul 27, 2018

The goal of this PR is to reduce the number of warnings and avoid what you can see here.

This should make Travis happy.

more information on #1565

@pep8speaks

This comment has been minimized.

Copy link

pep8speaks commented Jul 27, 2018

Hello @skoudoro, Thank you for updating !

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on July 27, 2018 at 17:58 Hours UTC
@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jul 27, 2018

LGTM! Let's see what that looks like on Travis.

@codecov-io

This comment has been minimized.

Copy link

codecov-io commented Jul 27, 2018

Codecov Report

Merging #1601 into master will increase coverage by 0.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1601      +/-   ##
==========================================
+ Coverage   87.32%   87.34%   +0.01%     
==========================================
  Files         246      246              
  Lines       31806    31814       +8     
  Branches     3450     3451       +1     
==========================================
+ Hits        27775    27787      +12     
+ Misses       3210     3205       -5     
- Partials      821      822       +1
Impacted Files Coverage Δ
dipy/reconst/tests/test_dsi_deconv.py 96.22% <100%> (+0.07%) ⬆️
dipy/reconst/tests/test_dsi.py 97.67% <100%> (+0.02%) ⬆️
dipy/tracking/life.py 97.8% <100%> (ø) ⬆️
dipy/reconst/tests/test_dsi_metrics.py 92% <100%> (+0.33%) ⬆️
dipy/reconst/dsi.py 81.42% <100%> (+0.1%) ⬆️
dipy/testing/__init__.py 81.81% <75%> (-1.52%) ⬇️
dipy/reconst/mapmri.py 90.93% <0%> (+0.64%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6e1da86...63c00e7. Read the comment docs.

@skoudoro

This comment has been minimized.

Copy link
Member

skoudoro commented Jul 27, 2018

Everything is green! Travis seems to be happy! it is ready to be merged @arokem

By default, Nosetest set up the warning filter as "always" which is the highest level. I switch it to "default" which means it will print the first occurrence of matching warnings for each location (module + line number) where the warning is issued

More info here

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jul 27, 2018

Yeah. I am not sure we don't want to just do that always. On the other hand, all these warnings are hard to ignore, so we end up doing something about them, before they overwhelm us.

Let's see if anyone else has anything to say about this. If no one pipes up, I will merge this on Monday.

@Garyfallidis

This comment has been minimized.

Copy link
Member

Garyfallidis commented Jul 28, 2018

I agree to merge. Thousands of repeated warning messages are really not useful. And we need to have master passing again, so that the other PRs can be rebased etc.
So, +1 on my side. Thanks.

@arokem

This comment has been minimized.

Copy link
Member

arokem commented Jul 28, 2018

Since we seem to have agreement, I will go ahead with the merge.

@arokem arokem merged commit b368e53 into nipy:master Jul 28, 2018

3 checks passed

codecov/patch 90.9% of diff hit (target 87.32%)
Details
codecov/project 87.34% (+0.01%) compared to 6e1da86
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@skoudoro skoudoro deleted the skoudoro:fix-excess-of-warning branch Jul 30, 2018

ShreyasFadnavis pushed a commit to ShreyasFadnavis/dipy that referenced this pull request Sep 20, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment