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

Release candidate for v1.0.0 #502

Merged
merged 25 commits into from Oct 19, 2023
Merged

Release candidate for v1.0.0 #502

merged 25 commits into from Oct 19, 2023

Conversation

sylvchev
Copy link
Member

@sylvchev sylvchev commented Oct 12, 2023

This is the update for MOABB v1.0.0 !

In this PR:

  • update moabb version to 1.0.0
  • fix versionadded mention in the doc
  • update the dependencies and poetry.lock file

I also updated the pyproject.toml to make it fully compatible with pip. It is now possible to install a local instance of moabb with:

git clone git@github.com:NeuroTechX/moabb.git
cd moabb
pip install -e .[carbonemission,deeplearning]

@sylvchev sylvchev added this to the 1.0.0 milestone Oct 12, 2023
@sylvchev sylvchev self-assigned this Oct 12, 2023
@sylvchev
Copy link
Member Author

@PierreGtch I modified the dependencies, but BIDS seems to be unchanged (at 0.13 version). I got a strange error when doing the unit test:

################## Pipeline: ####################
Pipeline(steps=[(<StepType.RAW: 'raw'>,
                 SetRawAnnotations(durations=3,
                                   event_id={'fake1': 1, 'fake2': 2,
                                             'fake3': 3})),
                (<StepType.RAW: 'raw'>,
                 FunctionTransformer(func=operator.methodcaller('filter', l_freq=7, h_freq=35, method='iir', picks='eeg', verbose=False)))])
################### Exception: ###################
Traceback (most recent call last):
  File "/home/runner/work/moabb/moabb/moabb/datasets/base.py", line 497, in _get_single_subject_data_using_cache
    interface.save(sessions_data)
  File "/home/runner/work/moabb/moabb/moabb/datasets/bids_interface.py", line 240, in save
    self._write_file(bids_path, obj)
  File "/home/runner/work/moabb/moabb/moabb/datasets/bids_interface.py", line 330, in _write_file
    mne_bids.write_raw_bids(
  File "<decorator-gen-637>", line 12, in write_raw_bids
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/mne_bids/write.py", line 2091, in write_raw_bids
    _write_raw_edf(raw, bids_path.fpath, overwrite=overwrite)
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/mne_bids/write.py", line 1156, in _write_raw_edf
    raw.export(bids_fname, overwrite=overwrite)
  File "<decorator-gen-241>", line 12, in export
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/mne/io/base.py", line 1761, in export
    export_raw(
  File "<decorator-gen-616>", line 12, in export_raw
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/mne/export/_export.py", line 75, in export_raw
    _export_raw(fname, raw, physical_range, add_ch_type)
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/mne/export/_edf.py", line 288, in _export_raw
    err = hdl.writeSamples(buf)
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/EDFlib/edfwriter.py", line 691, in writeSamples
    error = self.__write_edf_header()
  File "/home/runner/work/moabb/moabb/.venv/lib/python3.8/site-packages/EDFlib/edfwriter.py", line 879, in __write_edf_header
    self.__plus_patientcode = self.__plus_patientcode.strip()
AttributeError: 'int' object has no attribute 'strip'

Looks like when saving BIDS file (and writing it with EDF writer), the subject code is an int instead of a str. But the code seems to be good, applying subject_moabb_to_bids to make the conversion. Do you have an idea?

@PierreGtch
Copy link
Collaborator

This might have been introduced during a merge. I will investigate

@sylvchev
Copy link
Member Author

A regression during a merge seems unlikely, as the CI should have triggered the error as here. Except if there is a silent regression that was not catch by the test and that is now visible with the version update.

@PierreGtch
Copy link
Collaborator

Then it's probably due to change from EDFlib, mne or mne-bids; not sure which. I tried to fix the only possible cause I see.

@PierreGtch
Copy link
Collaborator

Seems to be fixed!

@sylvchev
Copy link
Member Author

the only error left is a file download error!

requests.exceptions.HTTPError: 500 Server Error: INTERNAL SERVER ERROR for url: https://zenodo.org/record/2605205/files/subject_01_VR.mat

I think we are on a good track to release MOABB v1.0.0 🎉 🔥 🐍 🧠

@sylvchev
Copy link
Member Author

@bruAristimunha Do you think we could try to solve the CI issue in BrainDecode and anticipate doc building issue with the dataset/session/runs name change?
The error when running Braindecode CI is :

in test/unit_tests/datasets/test_dataset.py

    def test_split_dataset(concat_ds_targets):
        concat_ds = concat_ds_targets[0]
        splits = concat_ds.split("run")
        assert len(splits) == len(concat_ds.description["run"].unique())
        [...]
        # Test split_ids as dict
        split_ids = dict(train=[1], test=[2])
        splits = concat_ds.split(split_ids)
        assert len(splits) == len(split_ids)
        assert splits.keys() == split_ids.keys()
>       assert (splits["train"].description["run"] == "run_1").all()
E       AssertionError: assert False
E        +  where False = <bound method NDFrame._add_numeric_operations.<locals>.all of 0    False\nName: run, dtype: bool>()
E        +    where <bound method NDFrame._add_numeric_operations.<locals>.all of 0    False\nName: run, dtype: bool> = 0    1\nName: run, dtype: object == 'run_1'.all   

It is likely due to the change in the run name (no underscore IIRC)

@bruAristimunha
Copy link
Collaborator

Yes @sylvchev! I will solve in braindecode :)

@sylvchev
Copy link
Member Author

CI for docs is also failling with download errors for several datasets (AlexMI, SSVEP-Exo, ...), so it seems not only related with Zenodo or Cattan2019_VR datasets.
Tracking down differences in updated package with this PR, urllib3 is the only one that was upgraded between this PR and previous ones (no version change in pooch, request or oath). The version of urllib raised from 1.* to 2.*
I'm trying to solve this by pinning version of urllib3 to version <= 2.0, it is temporary but I could not find any issue related to this on upstream.

@sylvchev
Copy link
Member Author

sylvchev commented Oct 16, 2023

With the example commented, we could merge now.
We need to solve the BIDS issue (@PierreGtch see #507 ) and the one on RV-BrainInvader for doc generation.

LGTM!

@sylvchev sylvchev merged commit 0f8b864 into NeuroTechX:develop Oct 19, 2023
7 of 8 checks passed
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.

None yet

4 participants