Improve create_from_X_y

[gemeinl]

split create_from_X_y into two functions, one returning BaseDatasets for further preprocessing on trial level, the second directly returning WindowsDatasets.

[agramfort]

why windows_datasets of type BaseConcatDataset and not WindowsDataset?

[gemeinl]

So probably naming is the problem here. In braindecode, a BaseDataset is a wrapper around a single mne.Raw. Similarly, a WindowsDataset is a wrapper around a single mne.Epochs. Both can be concatenated using the BaseConcatDataset class. So, since here multiple WindowsDatasets are returned, it is actually a BaseConcatDataset of WindowsDataset.

[agramfort]

thanks for the explanation. In mne when you concat raw you still get raw and if you concat epochs you still get epochs. Is it really necessary to have another type of dataset in the public API?

[gemeinl]

For the dataset part we tried to stick to the PyTorch API as closely as possible. Our BaseDataset inherits from torch.utils.data.dataset.Dataset and our BaseConcatDataset inherits from torch.utils.data.dataset.ConcatDataset, see https://pytorch.org/docs/stable/data.html#torch.utils.data.ConcatDataset. But we can always discuss with @robintibor.

[agramfort]

ok then I would just propose to update the docstring:

Suggested change

	windows_datasets: BaseConcatDataset
	concat_dataset: BaseConcatDataset

[agramfort]

that's an API breakage without deprecation

[gemeinl]

@agramfort Is it fine to use mne.utils.deprecated to add a DeprecationWarning? Or is this confusing, since the function will then appear to be a mne and not a braindecode function?

[agramfort]

i would copy such a util function in the braindecode somewhere.

[gemeinl]

Ok, thanks! I will discuss with @robintibor about where to put it and how to do it.

[agramfort]

would it make sense to keep a single create_from_X_y and then internally, dependings with items of X have the same length use windows or base? Basically as a user do I need to care of I can just pass the chunks of EEG data I have an the models will just deal with them? if you expect some breakage as some models only work with WindowsDataset it's another story but if you expect models to 'just work' maybe we can avoid some complexity on the user side.

[gemeinl]

Ok, I will try to explain what's going on. We just had a usecase (see #136) where a user had a dataset in Xy format, however, it was not preprocessed. So he wanted to convert it into braindecode format to be able to use the available braindecode/mne preprocessing. So far, the create_from_X_y function assumed data was already preprocessed and directly transformed it into compute windows for decoding. So it did not allow for preprocessing. I suggested something similar to you (see #136 (comment)), @robintibor did not agree. It is still undecided / open for discussion.

[agramfort]

to be honest rather than making complex the API of brain decode I would have written an example or referred the user to mne documentation to obtain raw instances and do his preprocessing properly. My worry is how are users going to discover the use cases for these functions? the more you have different ways to do the same thing the harder it is to document it. If there is only one prefered way it may force you to do an extra step but at least all scripts will look similar among users.

…

[gemeinl]

I totally see your points. @robintibor will be back on monday, we will then hopefully discuss all the PRs and issues.
Thank you very much for your valuable input @agramfort! I appreaciate it!

[robintibor]

Ok yes in the end maybe we settle towards our main example:

• starting from X,y
• explicitly going through mne (like creating raws etc.) in the example (noting this can be skipped if one already has mne data)
• then starting all braindecode from here

[robintibor]

closing this as it has diverged a bit much from current code, still very useful for addressing #544