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

[RFC] mne/channels structure #6724

Closed
massich opened this issue Aug 30, 2019 · 5 comments · Fixed by #6903
Closed

[RFC] mne/channels structure #6724

massich opened this issue Aug 30, 2019 · 5 comments · Fixed by #6903
Milestone

Comments

@massich
Copy link
Contributor

massich commented Aug 30, 2019

Lately with #6461 mne/channels is growing and I don't really like the structure it has. I would like to separate the readers in channels, the functions and the objects.

WDYT:

mne/channels
├── channels.py
├── data
│   ├── layouts
│   │   ├─    ....
│   │   └── Vectorview-mag.lout
│   ├── montages
│   │   ├─    ....
│   │   └── standard_primed.elc
│   └── neighbors
│       ├─    ....
│       └──  neuromag306planar_neighb.mat
├── _dig_montage_utils.py
├── _readers
│   ├── layout
│   └── montage
├── __init__.py
├── interpolation.py
├── layout.py
├── montage.py
└── tests
    ├── __init__.py
    ├── test_channels.py
    ├── test_interpolation.py
    ├── test_layout.py
    ├── test_montage.py
    └── test_standard_montage.py
mne/channels
├── channels.py
├── data
│   ├── layouts
│   │   ├─    ....
│   │   └── Vectorview-mag.lout
│   ├── montages
│   │   ├─    ....
│   │   └── standard_primed.elc
│   └── neighbors
│       ├─    ....
│       └──  neuromag306planar_neighb.mat
├── __init__.py
├── interpolation.py
├── layout.py
├── montage
│   ├── base.py
│   ├── _dig_montage_utils.py
│   ├── _standard_montage_loaders.py
│   └── _readers.py
├── montage.py
└── tests
    ├── __init__.py
    ├── test_channels.py
    ├── test_interpolation.py
    ├── test_layout.py
    ├── test_montage.py
    └── test_standard_montage.py

to have an init similar to:

from .layout import (Layout, make_eeg_layout, make_grid_layout,
                     find_layout, generate_2d_layout)
from .montage import (Montage, DigMontage,
                      get_builtin_montages, make_dig_montage)
from .channels import (equalize_channels, rename_channels, fix_mag_coil_types,
                       read_ch_connectivity, _get_ch_type,
                       find_ch_connectivity, make_1020_channel_selections)

from ._readers.montage import (
    read_dig_egi, read_dig_captrack, read_dig_fif, read_polhemus_fastscan,
    read_dig_polhemus_isotrak,
)

from ._readers.layout import read_layout

__all__ = [
    # Objects
    Layout, DigMontage,

    # Readers
    read_dig_egi, read_dig_captrack, read_dig_fif, read_polhemus_fastscan,
    read_dig_polhemus_isotrak, read_layout,

    # Factory methods
    make_dig_montage, make_grid_layout, make_eeg_layout, load_standard_montage,

    # Helper functions
    find_ch_connectivity,
]
@agramfort
Copy link
Member

agramfort commented Aug 30, 2019 via email

@larsoner larsoner added this to the 0.20 milestone Sep 21, 2019
@larsoner
Copy link
Member

@massich I was going to say that we should do this as soon as 0.20 is out, but then backporting any fixes will be much harder. So maybe this should be done in 0.19.

@larsoner larsoner modified the milestones: 0.20, 0.19 Sep 21, 2019
@massich
Copy link
Contributor Author

massich commented Sep 23, 2019

I was planning to do that after the release. And maybe some bugs pop up. So I would let it for afterwards. Backported if we have to, and have a nice clean code by the end of next week.

@larsoner
Copy link
Member

Okay it can wait

@larsoner larsoner modified the milestones: 0.19, 0.20 Sep 23, 2019
@massich
Copy link
Contributor Author

massich commented Sep 23, 2019

:) neither do I I've been willing to that for a LONG time :)
plus merge all the kit and others...

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 a pull request may close this issue.

3 participants