Add bi2012 dependency #37

gcattan · 2022-03-21T13:56:23Z

This PR add dependency to brain invaders 2012, so @toncho11 can use this data for the example in #36

toncho11 · 2022-03-24T08:52:41Z

@qbarthelemy can you please approve this PR, so that I can continue build on top of it?

sylvchev

Nice work. I have some questions regarding the mne dependency, but thank for the work! I did not know about the https://github.com/plcrodrigues/py.BI.EEG.2012-GIPSA !

sylvchev · 2022-03-24T09:31:04Z

doc/requirements.txt

@@ -2,7 +2,7 @@ sphinx-gallery
 sphinx-bootstrap_theme
 numpydoc
 cython
-mne[data]>=0.24
+mne==0.20.1


It is not a good idea to freeze the MNE version, it should really be avoided. Is this because of bi2012 dependency?

Unfortunately yes. It is using _fetch_file which doesn t seem available in recent versions of mne:

I have just opened an issue in bi2012, so we can keep trace of it.

doc/requirements.txt

Co-authored-by: Sylvain Chevallier <sylvain.chevallier@uvsq.fr>

gcattan · 2022-03-24T10:49:26Z

I think there is the same problem with fetch_dataset. bi2012 contains a duplicate of the download.py file of Moabb.
We likely need to apply the same fix as in Moabb inside bi2012, and then publish a new version of the package to pip.
But it will still be a workaround, because the problem is probably common to all of these repos, so we need to brainstorm a little bit probably.

toncho11 · 2022-03-24T11:12:11Z

Yes, I think it is better to proceed with this PR and fix properly the MNE dependency later.

qbarthelemy

Thanks for the contribution, but the example must be finished before merging.

pyriemann_qiskit/datasets/utils.py

examples/ERP/bi2012_quantum_classify.py

pyriemann_qiskit/datasets/utils.py

Add reference to bi2012 Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>

…iemann-qiskit into gc/bi2012_dependency

qbarthelemy

For pyRiemann-qiskit, can you validate PRs when they are ok for you? @gcattan

pyriemann_qiskit/datasets/utils.py

Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>

gcattan · 2022-03-25T17:33:30Z

@qbarthelemy Yes, of course! Thank you all for the review!

qbarthelemy

Thx @gcattan ! The ball is in your court @sylvchev.

sylvchev · 2022-03-25T18:47:05Z

Ok, I know that Pedro is not really available for now. Maybe the best approach could be to integrate BI in MOABB and to make it an optional dependency to pyriemann-qiskit.
I understand that this PR is important for moving to the next step, but I think it will results in a dependency hell if it is merged as is. MNE is pretty standard and it could results in important regression if someone want to try qiskit and it downgrades the MNE package.

I you left me a week, I could add the some of the BI datasets to MOABB and push a new release on pypi. What do you think @gcattan ?

gcattan · 2022-03-25T19:32:13Z

In general, I would agree with you, but we might end up with duplicates as it is the case between bi2013 in Moabb and in Pedro's repo. We could delete or deprecate the repos but Zenodo and the technical report are pointing to them (that said, a simple notice on the readme could do the trick).

May be a short-term compromise would be to fix mne dependency inside the bi2012 repo this week and migrate all necessary repo to Moabb after we have Pedro's input on this. How does that sound?

toncho11 · 2022-03-25T19:59:35Z

Actually having the Zenodo datasets on MOABB is a very good idea. This will finally create a single interface to all the EEG data we could use and test and will allow for the creation of models on a really large dataset (from many experiments/sets) which I hear is key to success. So adding BI 2012 to MOABB sounds good to me.

Then we will not longer use Pedro's code, but that is OK as long as it works.

gcattan · 2022-03-25T21:07:05Z

Ok, good point. @toncho11 I just saw the response from Pedro to your message. @sylvchev Let s go with Moabb then :)

sylvchev · 2022-03-29T08:39:54Z

I opened a PR on MOABB

gcattan · 2022-04-06T10:52:34Z

@sylvchev @toncho11 do we still need this PR if this is going to be integrated in moabb?

toncho11 · 2022-04-06T10:56:09Z

I guess no.
But now we depend on MOABB version > 0.4.5 (currently the one in the PR).

sylvchev · 2022-04-06T15:41:26Z

Agree, when new MOABB version will be available on PyPi, you will close this PR.

gcattan · 2022-04-07T06:11:31Z

Ok, thank you :) do you know when it will be online ?

sylvchev · 2022-04-07T12:29:35Z

It is live; just wait a few minutes that PyPi make the release of MOABB 0.4.6 effective.

gcattan · 2022-04-07T13:07:59Z

Super, thank you :)

gcattan added 21 commits March 20, 2022 19:32

add bi2012 dataset and util function

60d8dec

typo

326624c

add iterator over bi2012 dataset

c1cab2e

update documentation

4b99ad6

remove mne dependency

5a3c767

add same mne reference as in bi2012

8450f34

update conf.py

0a0e20b

workaround for datapath

96251c9

restart pipeline

e223a51

update workaround

58e9743

missing

5cb678e

try update_path

a39d982

missing

e490774

remove update_path

d594a1a

remove mne version

48bd549

try mne 0.20.1

02c3114

revert modif to utils.py related to data_path

c73df2e

make bi2012 a required dependency

e75bf18

lower mne version

75945c2

invalid syntax fixed

a863b8d

update index.rst

0e80456

sylvchev requested changes Mar 24, 2022

View reviewed changes

Update doc/requirements.txt

c786f54

Co-authored-by: Sylvain Chevallier <sylvain.chevallier@uvsq.fr>

qbarthelemy requested changes Mar 24, 2022

View reviewed changes

pyriemann_qiskit/datasets/utils.py Outdated Show resolved Hide resolved

examples/ERP/bi2012_quantum_classify.py Outdated Show resolved Hide resolved

pyriemann_qiskit/datasets/utils.py Show resolved Hide resolved

gcattan and others added 3 commits March 24, 2022 18:57

remove skeleton for bi2012_quantum_classify

1aa8cd9

Update pyriemann_qiskit/datasets/utils.py

256ed31

Add reference to bi2012 Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>

Merge branch 'gc/bi2012_dependency' of https://github.com/gcattan/pyR…

941fba5

…iemann-qiskit into gc/bi2012_dependency

gcattan added 2 commits March 24, 2022 20:05

refactor helper in utils.

a388976

update setup.py

9dd179f

qbarthelemy requested changes Mar 25, 2022

View reviewed changes

pyriemann_qiskit/datasets/utils.py Outdated Show resolved Hide resolved

pyriemann_qiskit/datasets/utils.py Outdated Show resolved Hide resolved

pyriemann_qiskit/datasets/utils.py Outdated Show resolved Hide resolved

gcattan and others added 3 commits March 25, 2022 18:32

Update pyriemann_qiskit/datasets/utils.py

aa8609e

Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>

Update pyriemann_qiskit/datasets/utils.py

97cc996

Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>

Update pyriemann_qiskit/datasets/utils.py

4ffd1a2

Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>

qbarthelemy approved these changes Mar 25, 2022

View reviewed changes

sylvchev mentioned this pull request Mar 29, 2022

Add Brain Invaders datasets NeuroTechX/moabb#283

Merged

gcattan closed this Apr 7, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add bi2012 dependency #37

Add bi2012 dependency #37

gcattan commented Mar 21, 2022

toncho11 commented Mar 24, 2022

sylvchev left a comment

sylvchev Mar 24, 2022

gcattan Mar 24, 2022

gcattan commented Mar 24, 2022

toncho11 commented Mar 24, 2022 •

edited

Loading

qbarthelemy left a comment

qbarthelemy left a comment

gcattan commented Mar 25, 2022

qbarthelemy left a comment

sylvchev commented Mar 25, 2022

gcattan commented Mar 25, 2022 •

edited

Loading

toncho11 commented Mar 25, 2022

gcattan commented Mar 25, 2022

sylvchev commented Mar 29, 2022

gcattan commented Apr 6, 2022

toncho11 commented Apr 6, 2022 •

edited

Loading

sylvchev commented Apr 6, 2022

gcattan commented Apr 7, 2022

sylvchev commented Apr 7, 2022

gcattan commented Apr 7, 2022

Add bi2012 dependency #37

Add bi2012 dependency #37

Conversation

gcattan commented Mar 21, 2022

toncho11 commented Mar 24, 2022

sylvchev left a comment

Choose a reason for hiding this comment

sylvchev Mar 24, 2022

Choose a reason for hiding this comment

gcattan Mar 24, 2022

Choose a reason for hiding this comment

gcattan commented Mar 24, 2022

toncho11 commented Mar 24, 2022 • edited Loading

qbarthelemy left a comment

Choose a reason for hiding this comment

qbarthelemy left a comment

Choose a reason for hiding this comment

gcattan commented Mar 25, 2022

qbarthelemy left a comment

Choose a reason for hiding this comment

sylvchev commented Mar 25, 2022

gcattan commented Mar 25, 2022 • edited Loading

toncho11 commented Mar 25, 2022

gcattan commented Mar 25, 2022

sylvchev commented Mar 29, 2022

gcattan commented Apr 6, 2022

toncho11 commented Apr 6, 2022 • edited Loading

sylvchev commented Apr 6, 2022

gcattan commented Apr 7, 2022

sylvchev commented Apr 7, 2022

gcattan commented Apr 7, 2022

toncho11 commented Mar 24, 2022 •

edited

Loading

gcattan commented Mar 25, 2022 •

edited

Loading

toncho11 commented Apr 6, 2022 •

edited

Loading