MRG: deprecated ConcatenateChannels and renamed EpochVectorizer #2361
Conversation
|
doc test don't pass because of deprecated call in the init... |
| @@ -45,6 +45,8 @@ Changelog | |||
|
|
|||
| - Add support to append new channels to an object from a list of other objects by `Chris Holdgraf`_ | |||
|
|
|||
| - Deprecated :class: ConcatenateChannels and replaced by :class: EpochVectorizer by `Romain Trachel`_ | |||
There was a problem hiding this comment.
EpochVectorizer -> EpochsVectorizer
There was a problem hiding this comment.
It also needs to be in backticks to render properly
|
besides LGTM |
|
Oops. Sorry guys, I merged to quick. On Wed, Jul 29, 2015 at 1:50 PM, Alexandre Gramfort <
|
| class ConcatenateChannels(TransformerMixin): | ||
| """Concatenates data from different channels into a single feature vector | ||
|
|
||
| Parameters | ||
| ---------- | ||
| info : instance of Info | ||
| The measurement info. |
There was a problem hiding this comment.
it didn't pass doc tests because of the call to deprecated function.
I found that removing parameters doc fix this issue
There was a problem hiding this comment.
Don't fix it this way, fix it by adding the function or class name here:
https://github.com/mne-tools/mne-python/blob/master/mne/tests/test_docstring_parameters.py#L64
|
ready to merge, if doc test and travis are happy |
trachelr
changed the title
|
ping me when you addressed @Eric89GXL comments |
|
done! thanks @Eric89GXL |
|
Yep because I get this error from test_docstring_parameters |
| @@ -147,6 +147,8 @@ def inverse_transform(self, epochs_data, y=None): | |||
| return X | |||
|
|
|||
|
|
|||
| @deprecated("Class 'ConcatenateChannels' has been renamed to " | |||
| "'EpochsVectorizer' and will be removed in release 0.11.") | |||
| class ConcatenateChannels(TransformerMixin): | |||
There was a problem hiding this comment.
can't you just do:
@deprecated("Class 'ConcatenateChannels' has been renamed to ...")
class ConcatenateChannels(EpochsVectorizer):
pass
in order to remove the old code of ConcatenateChannels?
There was a problem hiding this comment.
Yep, @Eric89GXL fix removed.
It seems to pass docstring test.
Thanks @agramfort !
|
perfect ! let's merge when travis is happy ! |
|
@trachelr for future reference you could have just added |
|
Got it @Eric89GXL ! |
MRG: deprecated ConcatenateChannels and renamed EpochVectorizer
|
thanks @trachelr |
|
@trachelr the deprecations must go to the API section of |
|
Ok! thanks @mainakjas |
@agramfort @dengemann This PR addresses issue #2313
Is it fine to import ConcatenateChannels as EpochVectorizer in the init ??
or I should copy the code of ConcatenateChannels and name it class EpochVectorizer?