mne-tools / mne-tools/mne-python
Export module inconsistent, needs API decision
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 3.5k
- Forks
- 1.6k
- Avg merge
- 1d 6h
- Merged PRs (30d)
- 100
Description
see https://github.com/mne-tools/mne-python/issues/10678#issuecomment-1139686322 and following 4 comments
In brief:
- the idea is to have one "export" function (and method) for raw, epochs, and evoked
- these functions delegate all the heavy lifting to functions that ideally are maintained and developed outside of mne (such as https://github.com/bids-standard/pybv) ... mne mostly contains a small wrapper around the external heavy lifting functions
- in some cases, these wrappers will be more involved (see EDF e.g., https://github.com/mne-tools/mne-python/blob/main/mne/export/_edf.py)
- ... and in some cases the "heavy lifting" function might be internal altogether (see MFF e.g., https://github.com/mne-tools/mne-python/blob/main/mne/export/_egimff.py)
- ideally though, the code in mne is minimal.
the problem
- some exporting funcs will accept additional parameters that are irrelevant to other exporting funcs
- for EDF this is done by adding these params to the general export function and noting only for EDF, see: https://mne.tools/stable/generated/mne.export.export_raw.html#mne.export.export_raw
- yet for MFF, the solution is to make the heavy lifting export function itself public, see: https://mne.tools/stable/generated/mne.export.export_evokeds_mff.html
this is inconsistent, and we should clean that up / make a consistent choice before the module continues to grow.
IMHO, the current "MFF way" is not a good style and would lead to lots of confusing public functions, instead of the clean "trinity" of raw, epochs, evoked .export. Couldn't we solve this by allowing **kwargs and documenting the kwargs that can be passed? Similar to how we do it in ICA when users pick picard or fastica, etc.
What are your opinions?
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Read the linked issue and its comment thread first, then compare the export interfaces and wrappers in mne/export/_edf.py and mne/export/_egimff.py, including export_raw and export_evokeds_mff. Done means the project has agreed on one consistent public export API and documented the decision before the module grows further.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api
- Issue type
- Refactor
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 25/100