Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improving feature names 2 #60

Merged
merged 5 commits into from Oct 19, 2020
Merged

Conversation

paulroujansky
Copy link
Contributor

Description

In the light of the changes made in PR #42 related to having more meaningful feature names when extract_features is called with return_df = True, I extended the mechanism to every univariate functions, as most of them didn't implement a get_feature_names method.

In order to do so, I added a very basic _compute_generic_feat_names function that returns a list [ch0, ch1, ...] (given input channel dim).
In addition to that, I added specific feature naming functions to the following metrics:

  • spect_edge_freq
  • spect_slope
  • wavelet_coef_energy
  • teager_kaiser_energy

As a consequence, the columns of the resulting dataframe are a bit more intelligible.
In addition to that, I added the possibility to pass the list of channel names when calling extract_features in order to translate these directly in the columns.

Example:

from mne_features.feature_extraction import extract_features
import numpy as np

data = np.random.random((5, 2, 500))

extract_features(data,
                 sfreq=250,
                 selected_funcs=['app_entropy', 'hurst_exp'],
                 ch_names=['PO3', 'PO4'],
                 return_as_df=True)

outputs

I added some tests to check these new functionnalities.

…s + enabling to specify channel names when calling extract_features() + adding tests
@@ -1097,6 +1160,9 @@ def compute_svd_entropy(data, tau=2, emb=10):
return -np.sum(np.multiply(sv_norm, np.log2(sv_norm)), axis=-1)


compute_svd_entropy.get_feature_names = _compute_generic_feat_names
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd rather do if not hasattra(function, 'get_feature_names') then use _compute_generic_feat_names. It avoids having to alter any possible function. Otherwise we need an OO design with a base class... which is a lot more work

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agreed, yet as all univariate functions do not necessarily return a (n_channels,)-shaped output (for instance compute_spect_slope), it could be equally dangerous to use a generic function directly in FeatureFunctionTransformer.fit(). As I see it, there are two approaches given the current implementation:

  • either define a function _compute_[funcname]_feat_names for every univariate functions and assign it (compute_[funcname].get_feature_names = _compute_[funcname]_feat_names), but then it would simpler /clearer to make it OO I guess)
  • assign a generic list of names in FeatureFunctionTransformer.transform() if the function is univariate and that the output channel dim is equal to the one of the input (has to be done in the transform() in order to get the shape of the output) <= I implemented this one in ed3ae2c, it's a bit messy though

@agramfort
Copy link
Member

looks clean to me. Can you please update https://github.com/mne-tools/mne-features/blob/master/doc/whats_new.rst with your name and a note on the contribution?

very glad to see this package useful to some !

@agramfort agramfort merged commit 42405e1 into mne-tools:master Oct 19, 2020
@agramfort
Copy link
Member

thx a lot @paulroujansky !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants