-
Notifications
You must be signed in to change notification settings - Fork 62
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 logging across scilpy #647
Conversation
Build passed ! Good Job 🍻 ! |
1 similar comment
Build passed ! Good Job 🍻 ! |
Ok so now the The idea is that we will never care about issues in scipy or numpy, but we might care about trimeshphy, commit or dipy, and we will always care about scilpy. |
Build passed ! Good Job 🍻 ! |
So the total output of tests on Travis is now less than 1000 (including the checkout, build, tests and warning) |
Build passed ! Good Job 🍻 ! |
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.
One comment but otherwise LGTM !
@@ -80,8 +79,7 @@ | |||
import numpy as np | |||
import nibabel as nib | |||
|
|||
from scilpy.io.streamlines import (lazy_streamlines_count, |
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.
Why is lazy_streamlines_count
no longer imported ? Was it unused before ?
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.
I am setting up my new computer and my autopep8 settings had too much power I think.
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.
I tested the pytest locally and, man!! Nice! So much easier! Takes a lot less time, too.
Thanks to Laurent I realized that
scil_search_keywords.py
stopped working.It was due to the fact that setting logging to
INFO
orDEBUG
the way we did (everywhere in Scilpy) does not work anymore.So it also meant that around 70% of our logging was not working properly (WARNING is fine, because it was the default for the
root::logging
).So I fixed it everywhere.
I also added a
pytest.ini
to print warnings from other libraries only once and keep all warnings from Scilpy (even if many) reducing printed line from 1M toPS: A few pep8 changes got in there while saving files, a few scripts didn't have permissions like everything else (777) so I changed it for uniformity.