mne-tools / mne-features

MNE-Features software for extracting features from multivariate time series
https://mne-tools.github.io/mne-features/
BSD 3-Clause "New" or "Revised" License
142 stars 32 forks source link

Fix #79 and remove some hard coded value in tests #80

Closed thomasfillon closed 2 years ago

thomasfillon commented 2 years ago

As discussed, here is my proposal for fixing #79.

I also change the retun value from log10 to dB (10 log10) and remove some hard coded power value in the corresponding test.

agramfort commented 2 years ago

Thx a lot @thomasfillon

can I ask you to add a line in https://github.com/mne-tools/mne-features/blob/master/doc/whats_new.rst to document the fix ?

also you should avoid opening PR from your master branch and use dedicated feature branches

πŸ™πŸ™πŸ™

thomasfillon commented 2 years ago

Thx a lot @thomasfillon

you're welcome @agramfort ;-)

can I ask you to add a line in https://github.com/mne-tools/mne-features/blob/master/doc/whats_new.rst to document the fix ?

Done. Is it ok for you ?

also you should avoid opening PR from your master branch and use dedicated feature branches

Sorry for that. Should I delete the PR and move the commits to a new feature branch ?

codecov[bot] commented 2 years ago

Codecov Report

Merging #80 (2dd66b5) into master (399092a) will increase coverage by 0.04%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master      #80      +/-   ##
==========================================
+ Coverage   93.53%   93.58%   +0.04%     
==========================================
  Files          10       10              
  Lines        1455     1465      +10     
==========================================
+ Hits         1361     1371      +10     
  Misses         94       94              
Impacted Files Coverage Ξ”
mne_features/tests/test_univariate.py 86.17% <100.00%> (+0.41%) :arrow_up:
mne_features/univariate.py 97.84% <100.00%> (ΓΈ)

Continue to review full report at Codecov.

Legend - Click here to learn more Ξ” = absolute <relative> (impact), ΓΈ = not affected, ? = missing data Powered by Codecov. Last update 399092a...2dd66b5. Read the comment docs.

agramfort commented 2 years ago

thx @thomasfillon πŸ™