IO library - Githubissues

mne-tools / mne-cpp

MNE-CPP: A Framework for Electrophysiology

https://mne-cpp.org/

BSD 3-Clause "New" or "Revised" License

152 stars 139 forks source link

IO library #483

Open LorenzE opened 4 years ago

LorenzE commented 4 years ago

With regard to PR #397 we should start thinking about how we want to support different file formats. I see three options right now:

Rename fiff library to io and include subfolders for all supported file types. Convert every non-fiff file format to Fiff internally so we can use it throughout MNE-CPP.
Create a new library for every format. Convert every non-fiff file format to Fiff internally so we can use it throughout MNE-CPP.
Introduce a more general data strucuture for internal MNE-CPP usage. This general data structure would then be able to interface with all other file formats, including Fiff. Currently, we have to convert everything to fiff file format if we want to use it in most MNE-CPP functions.

Last option is quite invasive and will lead to a considerable work load. Not sure if it's worth it.

juangpc commented 3 years ago

Hi, I want to clean up a little bit the pr list. I think I understand why this is stalled. My po is that the 3rd option you mention is the best one. However I think that pr shouldn't be kept there for that long. What do you guys think we should do with the pr?

Merge it.
Drop it, under the reasoning that we'll come back to it after we decide upon a long-term file format strategy. All related to the 3 options you comment.

LorenzE commented 3 years ago

Yes, Simon's PR has been around for some time now. I think the tool works and it could be used to convert fiff to edf as is. We wrote that tool for the BCH people to use. Maybe for now and until we know how to handle data, copy the branch to your private repo (just in case Simon deletes it at some point) and close the PR.

juangpc commented 3 years ago

Why not create a dev branch in this mne-cpp repository? I understand (and stand by) the idea of not over-using the branches, but they were created exactly for this situations. We push this work to a new branch "sheinke/io_lib_edf_to_fif" we leave it here so that the work can be re-captured later, eventually. Users not comfortable with a few branches > I don't think they'll ever get to see the branch at all. Users that know how to do a "git branch -va" > I don't think this will bother them. 👀 ?

LorenzE commented 3 years ago

The idea with the current approach ist not to pollute the main repository with not supported and unstable code which does not build. This is a matter of keeping the (central) main repo clean. Technically we already have a dev branch. The current master branch can be understood as our dev branch on which we work until we do a new release. New releases are then branched off if it was a major version release. For example if we would go from v0.1.8 to v.0.2.0 a new branch will be created. You can check out popular projects (mne-python, scikit-learn, etc.) which we used as an inspiration when we came up with MNE-CPP's release cycle early 2020.

Speaking of branches, we should maybe have this discussion for MNE-CPP as well: https://github.com/mne-tools/mne-python/issues/7901