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

including Liu2024 Dataset #619

Merged
merged 45 commits into from
Jul 8, 2024
Merged

including Liu2024 Dataset #619

merged 45 commits into from
Jul 8, 2024

Conversation

tahatt13
Copy link
Collaborator

@tahatt13 tahatt13 commented Jun 7, 2024

The dataset includes data from 50 acute stroke patients, and it involves two motor imagery tasks: right-handed and left-handed movements.
Here are more details about the dataset :

Name #Subj #Chan #Classes #Trials / class Trials len Sampling rate #Sessions
Liu2024 50 29 EEG 2 40 4 sec 500 Hz 1

@tahatt13 tahatt13 marked this pull request as draft June 7, 2024 14:53
@bruAristimunha bruAristimunha self-requested a review June 12, 2024 15:08
@bruAristimunha bruAristimunha marked this pull request as ready for review June 14, 2024 10:11
@bruAristimunha
Copy link
Collaborator

The question here is: is this dataset good enough to be merged?

super().__init__(
subjects=list(range(1, 50 + 1)),
sessions_per_subject=1,
events={"left_hand": 2, "right_hand": 4, "break": 3, "instr": 1},
Copy link
Collaborator

@bruAristimunha bruAristimunha Jun 14, 2024

Choose a reason for hiding this comment

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

Check the events

@bruAristimunha
Copy link
Collaborator

Hey @sebVelut!

Can you please review this code by next Friday? A shallow review is good!

@sebVelut
Copy link
Collaborator

sebVelut commented Jun 19, 2024

Hi !
I have reviewed and tested it by getting data with paradigm.get_data() and by testing WithinSessionEvaluation with a Xdawn + TS + LDA and everything worked without problems. The code seems good in my point of view.

But I think you should be careful about the events id described as said by Bruno, it should be the same as in the mapping.
Moreover, the use of annotations seems to work, another idea is to use a stim channel (or transform the existing one) and put the label of the events as value of the channel. Then the moabb will search for the stim channel first (this method seems to be taken by several other dataset implementation, ex: Physionet_mi or mpi_mi). Up to you if you decide to change the method.

A last point, in whats_new.rst, it is missing the line to say that you have added the Liu dataset

Copy link
Collaborator

Choose a reason for hiding this comment

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

Updating the line

Copy link
Collaborator

@PierreGtch PierreGtch left a comment

Choose a reason for hiding this comment

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

Thanks @tahatt13 ! Here are my comments

@@ -38,7 +38,7 @@ repos:
rev: 24.3.0
hooks:
- id: black
language_version: python3.8
language_version: python3
Copy link
Collaborator

Choose a reason for hiding this comment

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

@bruAristimunha is it ok to change the pre-commit file here?

docs/source/dataset_summary.rst Outdated Show resolved Hide resolved
Comment on lines 87 to 96
def __init__(self):
super().__init__(
subjects=list(range(1, 50 + 1)),
sessions_per_subject=1,
events={"left_hand": 1, "right_hand": 2, "instr": 3, "break": 4},
code="Liu2024",
interval=(2, 6),
paradigm="imagery",
doi="10.1038/s41597-023-02787-8",
)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I’m not sure if we should include the instruction and break events...
Because then by default, when using this dataset with the MotorImagery paradigm, algorithms would also have to classify these events.
Plus, will the instructions and break epochs really correspond to these phases? i.e. 4s interval, starting 2s after the event.

I suggest we don’t include them by default. And eventually we can leave the possibility to get them with dataset arguments:

Suggested change
def __init__(self):
super().__init__(
subjects=list(range(1, 50 + 1)),
sessions_per_subject=1,
events={"left_hand": 1, "right_hand": 2, "instr": 3, "break": 4},
code="Liu2024",
interval=(2, 6),
paradigm="imagery",
doi="10.1038/s41597-023-02787-8",
)
def __init__(self, break_events=False, instrumental_events=False):
events = {"left_hand": 1, "right_hand": 2}
if break_events:
events[“instr”] = 3
if insert_events:
events[“break”] = 4
super().__init__(
subjects=list(range(1, 50 + 1)),
sessions_per_subject=1,
events=events,
code="Liu2024",
interval=(2, 6),
paradigm="imagery",
doi="10.1038/s41597-023-02787-8",
)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I added that possibility. If I understood correctly what you meant, I think the epochs will be respected since the duration and onset are respected in the events file as you can see the break and instructions last for exactly 4seconds after each stimulu:
image

Copy link
Collaborator

Choose a reason for hiding this comment

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

The parameters break_events and inst_events are not working:

data = Liu2024(break_events=False, instr_events=False).get_data([1])
data[1]['0']['0'].annotations
#  <Annotations | 120 segments: break (40), instr (40), left_hand (20), ...>

( you still get annotations with all possible events even if you set these parameters to False)

The rest is great :)

moabb/datasets/liu2024.py Outdated Show resolved Hide resolved
moabb/datasets/liu2024.py Outdated Show resolved Hide resolved
moabb/datasets/liu2024.py Show resolved Hide resolved
moabb/datasets/liu2024.py Show resolved Hide resolved
PierreGtch and others added 2 commits July 8, 2024 22:11
Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Copy link
Collaborator

@PierreGtch PierreGtch left a comment

Choose a reason for hiding this comment

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

Thanks @tahatt13 :D

@PierreGtch PierreGtch enabled auto-merge (squash) July 8, 2024 20:20
@PierreGtch PierreGtch merged commit bb944ba into NeuroTechX:develop Jul 8, 2024
12 checks passed
PierreGtch added a commit to tahatt13/moabb that referenced this pull request Jul 10, 2024
* including Liu2024 Dataset

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Function data_path

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* data_infos and get_single_subject_data functions

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Data Description

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* updating get_single_subject fct and data_path & adding encoding fct

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Finishing the code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* updating docstrings for data_path

* Updating dataset_summary and updating the get_single_subject fct to handle the case of existing file in path_electrodes

* adapting the return of get_single_subject_data fct

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Adding dataset description and preload = True when reading the data in the get_single_subject fct

* fix: codespell

* fix: changing to static method the encoding

* repushing and resolving pre-commit conflicts

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* fix: changing the mapping

* fix: changing the unmatching between the trigger and the events from the csv

* ehn: using pylint to improve the code (remove not used variables, and change the module);

* modifying the python version in the pre-commit file

* adding description to enhancements

* adjusting the encoding

* adjusting comments and dataset description

* solving channels types and names issues & correcting encoding

* Correcting the number of trials per class and the total trials

* Correcting data description

* Adding the possibility to exclude/include break and instructions events

* Correcting code to include/exclude instr and break events

* Remove table from docstring

Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>

* Add CSV row

---------

Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: bruAristimunha <a.bruno@aluno.ufabc.edu.br>
Co-authored-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Co-authored-by: Pierre Guetschel <pierre.guetschel@donders.ru.nl>
bruAristimunha added a commit that referenced this pull request Sep 17, 2024
* Fix workflow cache (#632)

* Copy workflow changes from PR #584

* Update whats_new.rst

* Remove "always deploy" used for testing

* Restore change to pytest instead of unittest

* Use mne data cache in tests too

* Remove duplicate restore cache (#634)

* Fix example Hinss2021 (#636)

* Dataset summary in CSV and leaderboard links (#635)

* Add summary tables as csv

* Change BaseDataset to include the dataset's summary table in the docstring

* Try update summary table in doc

* Add test for dataset summary table

* Fix future annotations

* Prepare summary tables before doc

* Make build dir if not exist

* Add rstate table

* Replace tables with CSVs in doc

* Add PapersWithCOde column

* Fix initial white space

* Fix formatting of ints

* Remove tables from docstrings

* Add missing DemonsP300 row

* Update whats_new.rst

* Remove Shin2017B leaderboard

* Move PWC link outside of docstring table

* Add Liu2024 Dataset (#619)

* including Liu2024 Dataset

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Function data_path

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* data_infos and get_single_subject_data functions

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Data Description

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* updating get_single_subject fct and data_path & adding encoding fct

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Finishing the code

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* updating docstrings for data_path

* Updating dataset_summary and updating the get_single_subject fct to handle the case of existing file in path_electrodes

* adapting the return of get_single_subject_data fct

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Adding dataset description and preload = True when reading the data in the get_single_subject fct

* fix: codespell

* fix: changing to static method the encoding

* repushing and resolving pre-commit conflicts

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* fix: changing the mapping

* fix: changing the unmatching between the trigger and the events from the csv

* ehn: using pylint to improve the code (remove not used variables, and change the module);

* modifying the python version in the pre-commit file

* adding description to enhancements

* adjusting the encoding

* adjusting comments and dataset description

* solving channels types and names issues & correcting encoding

* Correcting the number of trials per class and the total trials

* Correcting data description

* Adding the possibility to exclude/include break and instructions events

* Correcting code to include/exclude instr and break events

* Remove table from docstring

Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>

* Add CSV row

---------

Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: bruAristimunha <a.bruno@aluno.ufabc.edu.br>
Co-authored-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Co-authored-by: Pierre Guetschel <pierre.guetschel@donders.ru.nl>

* Add scripts to publish results on PaperWithCode (#561)

* Add scripts to publish results on paperwithcode

* Create tasks manually instead (API error 403 forbidden)

* Try to create the evaluation first and other things, not working...

* Update whats_new.rst

* Fix example from API's README; still results in a "403 Forbidden"

* Fix task_id

* Allow passing multiple results files

* Save commands used as comments

* Add comment

---------

Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Signed-off-by: Bru <a.bruno@aluno.ufabc.edu.br>
Co-authored-by: Bru <a.bruno@aluno.ufabc.edu.br>

* [pre-commit.ci] pre-commit autoupdate (#631)

* [pre-commit.ci] pre-commit autoupdate

updates:
- [github.com/pre-commit/pre-commit-hooks: v4.5.0 → v4.6.0](pre-commit/pre-commit-hooks@v4.5.0...v4.6.0)
- [github.com/psf/black: 24.3.0 → 24.4.2](psf/black@24.3.0...24.4.2)
- [github.com/asottile/blacken-docs: 1.16.0 → 1.18.0](adamchainz/blacken-docs@1.16.0...1.18.0)
- [github.com/PyCQA/flake8: 7.0.0 → 7.1.0](PyCQA/flake8@7.0.0...7.1.0)
- [github.com/astral-sh/ruff-pre-commit: v0.3.5 → v0.5.0](astral-sh/ruff-pre-commit@v0.3.5...v0.5.0)
- [github.com/codespell-project/codespell: v2.2.6 → v2.3.0](codespell-project/codespell@v2.2.6...v2.3.0)

* FIX: including new word

* including

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Bru <a.bruno@aluno.ufabc.edu.br>

* Optuna GridSearch (#630)

* Optuna

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Optuna - Categorical Distibution

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Add optuna to dependency and regenerate poetry

* Add optuna to dependency and regenerate poetry

* Add optuna to dependency and regenerate poetry

* Add test on within Session

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* Add test on within Session

* Add test on within Session

* ehn: common function with dict

* ehn: moving function to util

* fix: correcting the what news file.

* Add test benchmark and raise an issue if the conversion didn't worked and exposed time_out parameter

* [pre-commit.ci] auto fixes from pre-commit.com hooks

* FIX: italian to eng

* EHN: making optuna optional

* FIX: fixing the workflow files

* FIX: changing the optuna file

* FIX: including optuna for the windows

---------

Signed-off-by: Bru <a.bruno@aluno.ufabc.edu.br>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: bruAristimunha <a.bruno@aluno.ufabc.edu.br>

* correct name (#642)

* V1.1.1 (#644)

* release 1.1.1

* increase the version

* increase the version

* removing the poetry.lock

* increase version

* changing the CITATION.cff

* upload versino for artifact

* pylock -> pyproject.toml

---------

Signed-off-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Signed-off-by: Bru <a.bruno@aluno.ufabc.edu.br>
Co-authored-by: PierreGtch <25532709+PierreGtch@users.noreply.github.com>
Co-authored-by: Taha Habib <95372659+tahatt13@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: Pierre Guetschel <pierre.guetschel@donders.ru.nl>
Co-authored-by: Igor Carrara <94047258+carraraig@users.noreply.github.com>
Co-authored-by: Quentin Barthélemy <q.barthelemy@gmail.com>
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.

4 participants