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

MRG: Add mne logger #112

Merged
merged 13 commits into from
Mar 22, 2022
Merged

MRG: Add mne logger #112

merged 13 commits into from
Mar 22, 2022

Conversation

mscheltienne
Copy link
Contributor

Closes #105.

I had this on my namespace for a while... (Oct 22. 2021) and I didn't take the time to finish it.
If I recall correctly, it's missing the ProgressBar class for the 10%... 20%... 30%... print output by the RANSAC (probably the most annoying one when I run this on 40 cores 😅).
I'll double-check the diff, make sure I didn't miss anything and add the ProgressBar for RANSAC.

Mathieu

@mscheltienne
Copy link
Contributor Author

Also have to fix a couple of logs, as it seems I just did a replace

@mscheltienne mscheltienne marked this pull request as draft March 21, 2022 21:52
@mscheltienne
Copy link
Contributor Author

mscheltienne commented Mar 21, 2022

image

Looks like this is working. And it is disabled if I change the loglevel of MNE.
Test are green local.

@mscheltienne mscheltienne marked this pull request as ready for review March 21, 2022 22:14
@mscheltienne mscheltienne changed the title Add mne logger MRG: Add mne logger Mar 21, 2022
@codecov-commenter
Copy link

codecov-commenter commented Mar 22, 2022

Codecov Report

Merging #112 (0046dff) into master (4a91d31) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #112      +/-   ##
==========================================
- Coverage   99.04%   99.02%   -0.02%     
==========================================
  Files           7        7              
  Lines         733      719      -14     
==========================================
- Hits          726      712      -14     
  Misses          7        7              
Impacted Files Coverage Δ
pyprep/utils.py 100.00% <ø> (ø)
pyprep/find_noisy_channels.py 100.00% <100.00%> (ø)
pyprep/ransac.py 97.85% <100.00%> (ø)
pyprep/removeTrend.py 95.08% <100.00%> (ø)

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 4a91d31...0046dff. Read the comment docs.

Copy link
Owner

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Please:

We can ignore the codecov check.
Previously there was some RNG issue in the tests, that has now disappeared ... still a bit weird - but in any case unrelated to your changes.

@mscheltienne
Copy link
Contributor Author

Done

@sappelhoff sappelhoff merged commit 0f41361 into sappelhoff:master Mar 22, 2022
@sappelhoff
Copy link
Owner

Thanks @mscheltienne

@mscheltienne mscheltienne deleted the add_mne_logger branch March 22, 2022 09:52
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.

Use the MNE logger to set the verbosity
3 participants