[MRG] Save preprocessing and windowing arguments

[gemeinl]

-initiated work on saving preprocessing and windowing arguments to BasseConcatDataset

[gemeinl]

As of PEP 3102 I would like to force all keyword arguments for Preprocessor. This avoids accidental misusage of apply_on_array, as well as it enables grabbing preprocessing parameters in a unified way to store them (we can simply grab kwargs).
Additionally, I would like to raise an error when a lambda function is passed to the preprocessor, since their arguments cannot be accessed in that way.

[codecov]

Codecov Report

Merging #287 (38a5b9b) into master (c20100c) will increase coverage by 0.54%.
The diff coverage is 98.24%.

@@            Coverage Diff             @@
##           master     #287      +/-   ##
==========================================
+ Coverage   79.56%   80.10%   +0.54%     
==========================================
  Files          49       49              
  Lines        2878     2931      +53     
==========================================
+ Hits         2290     2348      +58     
+ Misses        588      583       -5     

[gemeinl]

@hubertjb I just had to learn that we cannot use numpy.multiply with our current Preprocessor. The reason is that numpy.multiply requires two positional arguments (it actually forces two positional arguments using /, see https://numpy.org/doc/stable/reference/generated/numpy.multiply.html, https://www.python.org/dev/peps/pep-0570/) where our Preprocessor only supports keyword arguments (https://github.com/braindecode/braindecode/blob/master/braindecode/preprocessing/preprocess.py#L45). Maybe it is worth keeping braindecode.preprocessing.scale.

[hubertjb]

@hubertjb I just had to learn that we cannot use numpy.multiply with our current Preprocessor. The reason is that numpy.multiply requires two positional arguments (it actually forces two positional arguments using /, see https://numpy.org/doc/stable/reference/generated/numpy.multiply.html, https://www.python.org/dev/peps/pep-0570/) where our Preprocessor only supports keyword arguments (https://github.com/braindecode/braindecode/blob/master/braindecode/preprocessing/preprocess.py#L45). Maybe it is worth keeping braindecode.preprocessing.scale.

Interesting, I was not aware of pep570! And sorry about that, I should have been more careful. Let's maybe reinstate it but rename it braindecode.preprocessing.multiply, to avoid confusion with sklearn's scale and robust_scale?

[gemeinl]

Are there any other ideas for the structure of the saved preprocessing choices?
Right now they are consistently (for raw preprocessing, windowing and window preprocessing) stored as:
list(dict(str, dict(kwarg: value))).
So it would look like this:

[{'scale': {'factor': 1000000.0}}]

[{'create_windows_from_events': {'on_missing': 'error',
   'n_jobs': 1,
   'drop_bad_windows': True,
   'drop_last_window': False,
   'flat': None,
   'mapping': None,
   'picks': None,
   'preload': False,
   'reject': None,
   'trial_start_offset_samples': 0,
   'trial_stop_offset_samples': 0,
   'window_size_samples': 100,
   'window_stride_samples': 100}}]

[{'clip': {'mi': -800, 'ma': 800}},
 {'clip': {'a_min': -800, 'a_max': 800}},
 {'crop': {'tmin': 60, 'tmax': 180}},
 {'scale': {'factor': 1000000.0}},
 {'custom_crop': {'tmin': 30, 'tmax': 90, 'include_tmax': False}}]

Windowing kwargs are a special case with this structure. The list is not acually needed, since there will always be just one windowing function. I kept the list for consistency. Any suggestions for improvement? @robintibor @hubertjb @sliwy @agramfort ?

[gemeinl]

@hubertjb I just had to learn that we cannot use numpy.multiply with our current Preprocessor. The reason is that numpy.multiply requires two positional arguments (it actually forces two positional arguments using /, see https://numpy.org/doc/stable/reference/generated/numpy.multiply.html, https://www.python.org/dev/peps/pep-0570/) where our Preprocessor only supports keyword arguments (https://github.com/braindecode/braindecode/blob/master/braindecode/preprocessing/preprocess.py#L45). Maybe it is worth keeping braindecode.preprocessing.scale.

Interesting, I was not aware of pep570! And sorry about that, I should have been more careful. Let's maybe reinstate it but rename it braindecode.preprocessing.multiply, to avoid confusion with sklearn's scale and robust_scale?

Ah, no worries. I also was not aware. Yes we should avoid confusion with sklearn, although I am very unhappy about their choices here. Both with forcing positional arguments as well as with naming scale. I think standard_scale would be much more appropriate given that the other functions are called minmax_scale, robust_scale, ...

[gemeinl]

Maybe list(tuple(str, dict(kwarg: value))) would be more intuitive?

[gemeinl]

Ready to merge from my side unless there are objections.

[robintibor]

Please add description to whats_new

[robintibor]

thanks