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

Add Task Related Component Analysis (TRCA) #32

Merged
merged 21 commits into from
Apr 7, 2021

Conversation

gferraro2019
Copy link
Contributor

@gferraro2019 gferraro2019 commented Mar 23, 2021

Code based on the Matlab implementation from https://github.com/mnakanishi/TRCA-SSVEP

TODO:

  • write an example script
  • write a unit test
  • maybe refactor the code into a TRCA class with fit/predict methods. I think it be much more python-like.
  • make sure the documentation gets updated correctly

@nbara
Copy link
Owner

nbara commented Mar 23, 2021

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.

@codecov
Copy link

codecov bot commented Mar 23, 2021

Codecov Report

Merging #32 (73b52f0) into master (cee6af6) will increase coverage by 0.87%.
The diff coverage is 94.44%.

Impacted file tree graph

@@            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.

@nbara nbara changed the title add the Python translation of Masaki Nakanishi's TRCA and Filterbank Matlab tutorial Add Task Related Component Analysis (TRCA) Mar 23, 2021
@nbara nbara added the enhancement New feature or request label Mar 23, 2021
@nbara
Copy link
Owner

nbara commented Mar 23, 2021

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 :

  • write an example script
  • write a unit test
  • maybe refactor the code into a TRCA class with fit/predict methods. I think it be much more python-like.
  • make sure the documentation gets updated correctly

- actually integrate TRCA code into python package
- regroup trca utils
- update README
- fix PEP errors, improve docstrings all around
- draft example
meegkit/utils/trca.py Outdated Show resolved Hide resolved
@nbara
Copy link
Owner

nbara commented Mar 24, 2021

OK, I'm done I think. What do you think @gferraro2019 ?

@nbara
Copy link
Owner

nbara commented Mar 29, 2021

@gferraro2019 Did you have a chance to look at my changes? What do you think?

meegkit/utils/trca.py Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@gferraro2019
Copy link
Contributor Author

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.

@nbara
Copy link
Owner

nbara commented Apr 7, 2021

@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.

examples/example_trca.py Outdated Show resolved Hide resolved
@nbara nbara merged commit 52419ad into nbara:master Apr 7, 2021
@nbara
Copy link
Owner

nbara commented Apr 7, 2021

Thanks @gferraro2019 🎉

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

Successfully merging this pull request may close these issues.

Information about filter bank analysis (FBA) and task-related component analysis (TRCA)
2 participants