Closed gferraro2019 closed 3 years ago
Thanks for the contribution @gferraro2019!
FYI you can run make pep
from a terminal in the base project directory to run the linters, and pytest
to run the unit tests locally. But don't worry if the CIs fail and you're not too sure why. I can fix that myself later.
Merging #32 (73b52f0) into master (cee6af6) will increase coverage by
0.87%
. The diff coverage is94.44%
.
@@ Coverage Diff @@
## master #32 +/- ##
==========================================
+ Coverage 77.08% 77.95% +0.87%
==========================================
Files 18 20 +2
Lines 2055 2159 +104
==========================================
+ Hits 1584 1683 +99
- Misses 471 476 +5
Impacted Files | Coverage Δ | |
---|---|---|
meegkit/utils/stats.py | 57.34% <50.00%> (ø) |
|
meegkit/utils/trca.py | 91.42% <91.42%> (ø) |
|
meegkit/trca.py | 97.10% <97.10%> (ø) |
|
meegkit/__init__.py | 100.00% <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 cee6af6...73b52f0. Read the comment docs.
You actually added all the code outside of the meegkit
folder, so it will not checked by the CIs. I've made quite a few changes to cleanup the code and integrate it into the package.
It's looking good so far, but before we merge, I want to :
TRCA
class with fit/predict methods. I think it be much more python-like. OK, I'm done I think. What do you think @gferraro2019 ?
@gferraro2019 Did you have a chance to look at my changes? What do you think?
Hello, I'm sorry I couldn't look at the changes before but I had a family issue and I couldn't check them before.
I think you did a very great job. I added some comment in few parts.
I'm looking forward to hearing from you.
@gferraro2019 What do you think now? I've added a filterbank
parameters to the TRCA class, which allows passing custom filterbanks. The syntax is a bit heavier, but it's mostly due to the cheb1ord function itself so there isn't much I can do I think.
Thanks @gferraro2019 🎉
Code based on the Matlab implementation from https://github.com/mnakanishi/TRCA-SSVEP
TODO: