WIP: Revamps pyprep. An effort by NeurodataDesign at Johns Hopkins U #6

adam2392 · 2019-12-12T00:12:14Z

Addresses: #5

Fixes include:

full PREP implementation
SK-learn style fit() pattern
sub-pipelines as functions and classes, to allow customization
unit/integration tests of said files
black formatting
Revamped travis.CI
Includes notebook comparisons against MATLAB EEGLab's PREP

TODO:

update whats_new.rst
some additional testing using data I have w/ scalp EEG
clean up the raw data files to see if instead they can just be imported via mne-python (?)

@sappelhoff hope to get your initial feedback before tackling some of these "house-keeping" issues. Aamna and Liang are also done w/ the course at this point, so they may or may not help in this last endeavor. So I think between the two of us though, we can get this to however you want it to look :)

estimate of reference to median

Turn reference into class form

…into prep-dev

MRG: A common PR of Prep pipeline 1. Ports pyprep into Python3 2. Integrates full functionality of PREP 3. adds modularized sub-pipeline functions to run each part of PREP separately 4. Uses sklearn "fit()" function pattern

Remove read_montage function Use DigMontage instead of montage_kind in Prep class

Update MNE version requirement to 19.2 Replace read_montage function with make_standard_montage

codecov-io · 2019-12-15T22:38:13Z

Codecov Report

Merging #6 into master will decrease coverage by 1.68%.
The diff coverage is 97.69%.

@@            Coverage Diff             @@
##           master       #6      +/-   ##
==========================================
- Coverage     100%   98.31%   -1.69%     
==========================================
  Files           2       12      +10     
  Lines         322     1006     +684     
==========================================
+ Hits          322      989     +667     
- Misses          0       17      +17

Impacted Files	Coverage Δ
pyprep/noisy.py	`100% <100%> (ø)`	⬆️
test/test_removeTrend.py	`100% <100%> (ø)`
test/test_find_noisy_channels.py	`100% <100%> (ø)`
test/conftest.py	`100% <100%> (ø)`
test/test_reference.py	`100% <100%> (ø)`
test/test_prep_pipeline.py	`100% <100%> (ø)`
test/test_noisy.py	`100% <100%> (ø)`	⬆️
pyprep/prep_pipeline.py	`100% <100%> (ø)`
pyprep/removeTrend.py	`94.73% <94.73%> (ø)`
pyprep/utilities.py	`94.73% <94.73%> (ø)`
... and 12 more

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 0ef59ed...5f66193. Read the comment docs.

sappelhoff

Thanks!

I played around a bit with the changes by pulling your branch and making a new conda environment for it.

Please note that all of my comments are only "nice-to-have", because I lack the time to actually help with the requests :)

The prep_demo.ipynb does not run "out of the box" because it has a wrong path for the example file
- would be nice to link to this example from the website in the form of a sphinx gallery like on MNE-BIDS. https://github.com/mne-tools/mne-bids/blob/0aff75c0d02a1c84925a7a212659cff1c01d3f29/doc/conf.py#L119 ... in the future we could then even make this example interactive using binder (similar to how it's done on mne-bids)
Running the test suite, I get 39 warnings, most of which are deprecation warnings. It would be worth looking into resolving these warnings
the Makefile contains some non-working commands ... e.g., the build-doc command. I suggest to remove all unnecessary commands, keep the file minimal, and fix what's broken
Travis does not seem to run on this PR - any idea why?

adam2392 · 2019-12-16T16:45:48Z

The prep_demo.ipynb does not run "out of the box" because it has a wrong path for the example file

would be nice to link to this example from the website in the form of a sphinx gallery like on MNE-BIDS. https://github.com/mne-tools/mne-bids/blob/0aff75c0d02a1c84925a7a212659cff1c01d3f29/doc/conf.py#L119 ... in the future we could then even make this example interactive using binder (similar to how it's done on mne-bids)

Yes I agree, the datasets should be sort of "imported" via MNE API. Should be easily fixed here.

Can you advise what you mean for linking it in the sphinx gallery? Is all that is necessary, an explicit reference in that conf.py file? If not, what else should we do?

Running the test suite, I get 39 warnings, most of which are deprecation warnings. It would be worth looking into resolving these warnings

Agreed. I think these are mainly from the montage API being overhauled in the MNE versions.

the Makefile contains some non-working commands ... e.g., the build-doc command. I suggest to remove all unnecessary commands, keep the file minimal, and fix what's broken

Can erase as necessary.

Travis does not seem to run on this PR - any idea why?

I believe this should be a setting on your end? I am not a maintainer on this repo, so I can't access the travis, but I think there's a setting that enables PRs/branches to have travis run? If not, I'm not sure...

sappelhoff · 2019-12-17T09:32:25Z

I believe this should be a setting on your end? I am not a maintainer on this repo, so I can't access the travis, but I think there's a setting that enables PRs/branches to have travis run? If not, I'm not sure...

I just checked, and Travis is running the PR ... it's just not showing on GitHub somehow. Perhaps that'll be fixed automatically once one more commit is pushed.

I am not a maintainer on this repo

I just made you a maintainer, thanks again for contributing together with Aamna and Liang.

Can you advise what you mean for linking it in the sphinx gallery? Is all that is necessary, an explicit reference in that conf.py file? If not, what else should we do?

yes, the conf.py file needs to be edited to contain a sphinx-gallery import and the settings ... and more importantly, the ipynb example would have to be converted to a py file.

Some links to show you what I mean:

importing sphinx-gallery in conf.py
- https://github.com/mne-tools/mne-bids/blob/4234280958775c445861d2c93b2b41d5c040391a/doc/conf.py#L6
- https://github.com/mne-tools/mne-bids/blob/4234280958775c445861d2c93b2b41d5c040391a/doc/conf.py#L44
making sphinx-gallery settings in conf.py
- https://github.com/mne-tools/mne-bids/blob/4234280958775c445861d2c93b2b41d5c040391a/doc/conf.py#L119-L135
- not that the "binder" settings can be ignored for now
converting the ipynb to a python/rst format
- https://github.com/mne-tools/mne-bids/blob/master/examples/convert_eeg_to_bids.py
- basically, the markdown cells of the ipynb become commented "blocks" in the py/rst file

adam2392 · 2019-12-23T04:51:00Z

@sappelhoff besides codecov, things are working out.

I converted the jupyter notebook to .py files, and did some adaptation of the conf.py files per your suggestion. I'm not too experienced w/ making docs the way you all do at MNE, but if there are still things missing, let me know!

Happy holidays ;)

sappelhoff

hi @adam2392 thanks for your patience :-)

If you could:

make the CIs pass
update what's new (https://github.com/sappelhoff/pyprep/blob/master/docs/source/whats_new.rst)

then I can merge this PR and deal with the rest.

adam2392 · 2020-01-06T21:02:31Z

hi @adam2392 thanks for your patience :-)

If you could:

make the CIs pass

update what's new (https://github.com/sappelhoff/pyprep/blob/master/docs/source/whats_new.rst)

then I can merge this PR and deal with the rest.

Fixed the bug that pings users for input in the pytest. Updated the testing to make use of some fixtures that get thrown back and forth (e.g. mne.io.Raw and mne.Montage).

sappelhoff · 2020-01-07T10:56:48Z

and it's in! Thanks a lot everybody! :-)

May it help future researchers!

sappelhoff · 2020-02-10T15:05:50Z

@adam2392 I now fully integrated the code and updated the docs (see bottom of examples page: https://pyprep.readthedocs.io/en/latest/examples.html)

I decided against using binder + sphinx gallery for now, and instead I just ran the notebook on my machine, uploaded the ipynb output and provided a link to a notebook viewer:

https://nbviewer.jupyter.org/github/sappelhoff/pyprep/blob/master/examples/prep_demo.ipynb

Interestingly, the results now don't match so well anymore. Check the "results" section of the notebook, comparing the pyprep output and the "Matlab output".

I did not change any crucial code. What I did was:

fix some links
account for the fact that prep_pipeline takes a montage, not a string
introduce a more complete channel renaming function ... the map that you used before did not completely work --> I don't know how you didn't get errors before ... perhaps these errors were only recently introduced in MNE 🤷‍♂️

Anyhow, before we can make a release, we should check the full prep pipeline's output again.

Until that happens, I think it's okay to have this as an unreleased "work in progress" feature, which is also fine :-)

Nick3151 and others added 30 commits November 25, 2019 18:10

Add robust reference and test functions

6edfe60

Updated noisy channels

983a3ff

Filtering/Detrending EEG data

9166bd6

update noisy.py

c8007cc

updated noisy channel detection

463b9a2

filtering/detrending EEG data

0cdde47

test for finding noisy channels

cc54f33

test for removing/filtering the EEG data

f582efe

Recover high pass filtering, change default initial

fca2431

estimate of reference to median

Merge remote-tracking branch 'origin/AamnaDev' into prep-dev

db6c2b4

updated test for noisy channels

abfaf59

updated test for removing trend

3604071

Merge branch 'AamnaDev' into prep-dev

f9767fb

New test for noisy channels

ab4c41d

New test for filtering/detrending

dc2bf88

Remove test for noisy channels

6a74837

Removing test for detrending

4cad059

minor changes in noisy channels

648fb21

minor changes in noisy channels

b55c17f

updated test

0b57a5c

adding additional argument value to quantile function

0fc9375

Add prep_pipeline(fit style)

891017c

Turn reference into class form

Update reference.py

3adbf1d

Merge branch 'prep-dev' of https://github.com/NeuroDataDesign/pyprep …

4e2d589

…into prep-dev

Updata docstring of prep_pipeline

c2e6a71

Add Physionet test data

d69f335

Update test functions

b76e546

Update travis.yml to Python3.6

f308a66

Update travis.yml

32e5222

Update test functions

4988f82

Nick3151 and others added 10 commits December 11, 2019 17:18

Bug fixing

0ef91d7

Add jupyter notebook and Matlab's results

8e5f29e

Test bug fixing

6e3a470

Added random seed init to stefan's function.

3130a4a

Add test_prep_pipeline.py

71b7607

Merge branch 'prep-dev' of https://github.com/NeuroDataDesign/pyprep …

8d0bf31

…into prep-dev

test_findnoisychannels bug fixing

e7a7a5a

Merge pull request #4 from NeuroDataDesign/prep-dev

d1ec4ec

MRG: A common PR of Prep pipeline 1. Ports pyprep into Python3 2. Integrates full functionality of PREP 3. adds modularized sub-pipeline functions to run each part of PREP separately 4. Uses sklearn "fit()" function pattern

Update MNE version requirement to 19.2

a840bae

Remove read_montage function Use DigMontage instead of montage_kind in Prep class

Merge pull request #6 from NeuroDataDesign/prep-dev

420d373

Update MNE version requirement to 19.2 Replace read_montage function with make_standard_montage

sappelhoff reviewed Dec 16, 2019

View reviewed changes

adam2392 added 2 commits December 20, 2019 11:48

Cleaned up prep demo into python file and added sphinx updating gallery.

a936ae1

Adding black formatted.

ef64430

adam2392 requested a review from sappelhoff December 22, 2019 01:28

adam2392 added the enhancement New feature or request label Dec 22, 2019

Adding mne API for loading raw example data.

7c5b07f

sappelhoff reviewed Jan 6, 2020

View reviewed changes

adam2392 added 3 commits January 6, 2020 14:40

Adding conftest and also updating whats_new.

2ebc18a

Adding dataset for travis.yml.

0c1431d

Make user prompt disabled.

5f66193

sappelhoff merged commit f97e7ec into sappelhoff:master Jan 7, 2020

sappelhoff mentioned this pull request Feb 6, 2020

Implementation of Full PREP and Python3 compatability #5

Closed

sappelhoff mentioned this pull request Apr 4, 2020

noisy.py module should be removed #11

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: Revamps pyprep. An effort by NeurodataDesign at Johns Hopkins U #6

WIP: Revamps pyprep. An effort by NeurodataDesign at Johns Hopkins U #6

adam2392 commented Dec 12, 2019 •

edited by sappelhoff

Loading

codecov-io commented Dec 15, 2019 •

edited

Loading

sappelhoff left a comment

adam2392 commented Dec 16, 2019

sappelhoff commented Dec 17, 2019

adam2392 commented Dec 23, 2019

sappelhoff left a comment

adam2392 commented Jan 6, 2020

sappelhoff commented Jan 7, 2020

sappelhoff commented Feb 10, 2020

WIP: Revamps pyprep. An effort by NeurodataDesign at Johns Hopkins U #6

WIP: Revamps pyprep. An effort by NeurodataDesign at Johns Hopkins U #6

Conversation

adam2392 commented Dec 12, 2019 • edited by sappelhoff Loading

codecov-io commented Dec 15, 2019 • edited Loading

Codecov Report

sappelhoff left a comment

Choose a reason for hiding this comment

adam2392 commented Dec 16, 2019

sappelhoff commented Dec 17, 2019

adam2392 commented Dec 23, 2019

sappelhoff left a comment

Choose a reason for hiding this comment

adam2392 commented Jan 6, 2020

sappelhoff commented Jan 7, 2020

sappelhoff commented Feb 10, 2020

adam2392 commented Dec 12, 2019 •

edited by sappelhoff

Loading

codecov-io commented Dec 15, 2019 •

edited

Loading