Add "dbs" as new channel type #8739

richardkoehler · 2021-01-12T21:35:59Z

Related to #8681

What does this implement/fix?

Add "dbs" as new channel type for recordings obtained from deep brain stimulation (DBS) electrodes.
The channel type DBS is referenced to in iEEG-BIDS (https://www.nature.com/articles/s41597-019-0105-7

agramfort

some brief comments

@richardkoehler do you have some public data we could use for a tutorial? that would be a good way to test that it works for end users

doc/changes/latest.inc

agramfort · 2021-01-13T08:44:39Z

doc/changes/names.inc


 .. _Qianliang Li: https://www.dtu.dk/english/service/phonebook/person?id=126774
+
+.. _Richard Koehler: https://github.com/richardkoehler


missing empty line

agramfort · 2021-01-13T08:45:08Z

mne/channels/channels.py

        ch_names, chs = self.info['ch_names'], self.info['chs']
        picks = pick_types(self.info, meg=False, eeg=True,
-                           seeg=True, ecog=True)
+                           seeg=True, dbs=True, ecog=True)


put the dbs at the end not just after seeg everywhere.

Thanks, will do!

agramfort · 2021-01-13T08:46:08Z

mne/channels/channels.py

-                   seeg=False, dipole=False, gof=False, bio=False, ecog=False,
-                   fnirs=False, csd=False, include=(), exclude='bads',
-                   selection=None, verbose=None):
+                   seeg=False, dbs=False, dipole=False, gof=False, bio=False,


dbs just be added just before include to avoid risk of crash if someone passed parameters without names

agramfort · 2021-01-13T08:47:14Z

mne/cov.py

+    def __init__(self, info, grad=0.1, mag=0.1, eeg=0.1, seeg=0.1, dbs=0.1,
+                 ecog=0.1, hbo=0.1, hbr=0.1, fnirs_cw_amplitude=0.1,
                 fnirs_fd_ac_amplitude=0.1, fnirs_fd_phase=0.1, fnirs_od=0.1,
                 csd=0.1, store_precision=False, assume_centered=False):


same here. You cannot insert a parameter in a list without breaking API unless you put it at the end or you enforce named parameters.

agramfort · 2021-01-13T08:48:06Z

mne/io/array/tests/test_array.py

    # plotting
    raw2.plot()
-    raw2.plot_psd(tmax=2., average=True, n_fft=1024, spatial_colors=False)
+    raw2.plot_psd(picks=['meg','eeg'], tmax=2., average=True, n_fft=1024,


why did you have to change this?

I was getting an Error during testing, but it was my bad, I revoked this change !

mne/io/constants.py

richardkoehler · 2021-01-13T09:43:40Z

some brief comments

@richardkoehler do you have some public data we could use for a tutorial? that would be a good way to test that it works for end users

@agramfort Thanks for the comments! I'm working on smoothing out the remaining issues and I will implement your suggestions. I wasn't able to run a full test on my computer, as I kept getting Bus Errors for certain tests (e.g. mne/io/tests/test_pick.py::test_pick_forward_seeg_ecog). I'm happy to provide some public data. Do you want to integrate this in the tests as well or only use it for a tutorial?

Adedd gh pull number Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>

larsoner · 2021-01-13T13:18:43Z

Also note that in #8736 we found a place where seeg was missing, you should check to see if you need a similar change

I'm happy to provide some public data. Do you want to integrate this in the tests as well or only use it for a tutorial?

For tests I think it's enough just to fake DBS data using inst.set_channel_types EDIT: or RawArray with create_info(..., 'dbs')`. Having a tutorial "Working with DBS data" would be cool!

richardkoehler · 2021-01-13T13:59:28Z

Also note that in #8736 we found a place where seeg was missing, you should check to see if you need a similar change

I'm happy to provide some public data. Do you want to integrate this in the tests as well or only use it for a tutorial?

For tests I think it's enough just to fake DBS data using inst.set_channel_types EDIT: or RawArray with create_info(..., 'dbs')`. Having a tutorial "Working with DBS data" would be cool!

Thanks, I added "dbs" to topomap.py as "seeg" in #8736 . I will make a tutorial with DBS data as soon as I have the rest of the PR finalized.

richardkoehler · 2021-01-14T08:50:49Z

@larsoner @agramfort
So, the PR finally passed all checks, although I'm not really sure the tests I added are done the way they should; I tried my best.
Concerning the tutorial, I would need to upload the data to make it publically availabe right? I am already in contact with our data protection office in order to meet all the requirements, but the process might take a while because we might need patient consent. I will also double check if there are any publically available DBS LFP datasets.

larsoner · 2021-01-14T15:12:50Z

Concerning the tutorial, I would need to upload the data to make it publically availabe right?

Yes, typically we upload to osf.io though you can use other providers if you want.

larsoner

Otherwise LGTM, that's a lot of changes!

I think you need a rebase or merge with upstream/master @richardkoehler . Let me know if you need help with it

mne/io/constants.py

mne/io/tests/test_pick.py

mne/viz/_3d.py

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>

agramfort

let's address the remaining comments from @larsoner and we're good to go!

thx @richardkoehler

doc/changes/latest.inc

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

larsoner · 2021-01-15T21:24:59Z

In it goes, thanks @richardkoehler !

* upstream/master: (66 commits) MRG, ENH: Add infant template downloader (mne-tools#8738) ENH: add reader for NeuroElectrics .nedf files (mne-tools#8734) DOC: improve glossary entry about fiducials (mne-tools#8763) MRG, ENH: Add Report.add_custom_css (mne-tools#8762) BUG, DOC: read_raw_egi didn't support pathlib.Path; update read_raw() docstring (mne-tools#8759) Add "dbs" as new channel type (mne-tools#8739) MRG, VIZ: Fix title position in plot_sensors (mne-tools#8752) MRG: Support for non-FIFF files in Report.parse_folder (mne-tools#8744) MRG, VIZ, FIX: sEEG picking in _prepare_topomap_plot() (mne-tools#8736) DOC: don't use single letter variable name in _compute_forward (mne-tools#8727) WIP: Fix search [skip github] [skip azp] (mne-tools#8742) WIP: Compare Beer-lambert to HOMER (mne-tools#8711) MRG: bump spyder version (mne-tools#8020) FIX anon with IO round trip (mne-tools#8731) fix set_bipolar_reference for Epochs (mne-tools#8728) WIP: Add width argument, reduce default (mne-tools#8725) ENH: Add toggle-all button to Report (mne-tools#8723) fix int/float conversion in nicolet header (mne-tools#8712) MRG, BUG: Fix Report.add_bem_to_section n_jobs != 1 (mne-tools#8713) MRG, DOC: Make "rank" options in docs more accessible (mne-tools#8707) ...

richardkoehler added 6 commits January 8, 2021 10:13

Add current commit fiff-constants to test_constants.py

d71dcec

Add DBS as channel type

ee14bb1

Add DBS as channel type

d02b00c

Add DBS as channel type

d16c2e5

Add "dbs" as new channel type

387b051

Update latest.inc

5a1e7ef

richardkoehler mentioned this pull request Jan 12, 2021

Add DBS (deep brain stimulation) as new channel type #8710

Closed

agramfort reviewed Jan 13, 2021

View reviewed changes

richardkoehler and others added 2 commits January 13, 2021 10:55

Update doc/changes/latest.inc

62c91bb

Adedd gh pull number Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>

Fix tests and API compatibility

44d16c1

Add dbs to topomap and bugfixes

610e9cd

richardkoehler added 3 commits January 13, 2021 15:02

Update test_topo.py

6d7836d

bugfix documentation and added test

3603167

Bugfix tests

d76dda5

larsoner reviewed Jan 14, 2021

View reviewed changes

mne/io/constants.py Outdated Show resolved Hide resolved

mne/io/tests/test_pick.py Outdated Show resolved Hide resolved

mne/io/tests/test_pick.py Outdated Show resolved Hide resolved

mne/viz/_3d.py Outdated Show resolved Hide resolved

Add missing space to mne/io/constants.py

9200eda

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org>

agramfort reviewed Jan 14, 2021

View reviewed changes

doc/changes/latest.inc Outdated Show resolved Hide resolved

richardkoehler and others added 4 commits January 15, 2021 08:24

Merge remote-tracking branch 'upstream/master'

ad27925

Apply suggestions from code review

8fca9b3

Co-authored-by: Alexandre Gramfort <alexandre.gramfort@m4x.org> Co-authored-by: Eric Larson <larson.eric.d@gmail.com>

Update test_pick.py

65f8a4c

Merge branch 'master' of https://github.com/richardkoehler/mne-python

2becb1c

agramfort approved these changes Jan 15, 2021

View reviewed changes

Merge remote-tracking branch 'upstream/master'

0b26103

larsoner merged commit e4fcd77 into mne-tools:master Jan 15, 2021

larsoner mentioned this pull request Feb 19, 2021

New channel type: DBS #8681

Closed


		.. _Qianliang Li: https://www.dtu.dk/english/service/phonebook/person?id=126774

		.. _Richard Koehler: https://github.com/richardkoehler No newline at end of file

Uh oh!

Add "dbs" as new channel type #8739

Add "dbs" as new channel type #8739

Uh oh!

Conversation

richardkoehler commented Jan 12, 2021

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

agramfort Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

agramfort Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

richardkoehler Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

agramfort Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

agramfort Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

agramfort Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

richardkoehler Jan 13, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

richardkoehler commented Jan 13, 2021

Uh oh!

larsoner commented Jan 13, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

richardkoehler commented Jan 13, 2021

Uh oh!

richardkoehler commented Jan 14, 2021

Uh oh!

larsoner commented Jan 14, 2021

Uh oh!

larsoner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

agramfort left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

larsoner commented Jan 15, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

larsoner commented Jan 13, 2021 •

edited

Loading