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

BUG: Fix URL #600

Merged
merged 10 commits into from
Sep 10, 2022
Merged

BUG: Fix URL #600

merged 10 commits into from
Sep 10, 2022

Conversation

larsoner
Copy link
Member

@larsoner larsoner commented Sep 9, 2022

Before merging …

  • Add config for 4107
  • Add test that all tests show up in CircleCI config
  • Add 4107 and whatever else we need to tests
  • Changelog has been updated (docs/source/changes.md)

Okay the basic fix was easy. Draft because I'm not sure how this download has ever worked, as this error appears in the blame from 2 years ago when it was added. It appears not to be used on CircleCI -- should it be? If so, I'll add another little sanity check test here that any dataset in datasets.py has at least some matching code in .circleci/config.yml.

Closes #599
Closes #539

@larsoner
Copy link
Member Author

larsoner commented Sep 9, 2022

I'll probably take care of #539 here as well since it should be easy enough hopefully

tests/run_tests.py Outdated Show resolved Hide resolved
hoechenberger added a commit to hoechenberger/openneuro-py that referenced this pull request Sep 10, 2022
I just glanced at the errant filename for
mne-tools/mne-bids-pipeline#600, it only became
glaringly obvious what the error was once I offered some suggestions:
```
RuntimeError: Could not find path in the dataset:
- sub-001/eeg/sub-001_task-AudioCueWalkingStudy_run-01_events.tsvsub-001/eeg/sub-001_task-AudioCueWalkingStudy_run-01_eeg.set
Perhaps you mean one of these paths:
- sub-001/eeg/sub-001_task-AudioCueWalkingStudy_run-01_events.tsv
- sub-001/eeg/sub-001_task-AudioCueWalkingStudy_run-14_events.tsv
- sub-001/eeg/sub-001_task-AudioCueWalkingStudy_run-13_events.tsv
Please check your includes.
```
This PR makes these suggestions.

Co-authored-by: Richard Höchenberger <richard.hoechenberger@gmail.com>
@agramfort
Copy link
Member

Personally I prefer this dryer option. +1 merge after the CIs are green. Note that the new dataset shall be added to the documentation page too.

@larsoner
Copy link
Member Author

larsoner commented Sep 10, 2022

I think there is a bug with 1971 with participants.tsv:


participant_id  participant_id  sex     age     handedness
sub-001 CQ2     F       18-38   R
sub-002 BT4     F       28      R
sub-003 CQ1     F       18-38   R
sub-004 CQ3     F       18-38   R
sub-005 CP9     M       18-38   R
sub-006 CA7     M       18-38   R
sub-007 CB6     M       32      R
sub-008 BV2     M       38      R
sub-009 CP8     F       18-38   R
sub-010 CP1     F       23      R
sub-011 CP2     M       18-38   R
sub-012 CO9     F       18-38   R
sub-013 CP5     M       18-38   R
sub-014 CL1     M       18-38   R
sub-015 BL7     M       28      R
sub-016 CO3     M       18-38   R
sub-017 CE2     M       18-38   R
sub-018 CV6     F       18-38   R
sub-019 CJ2     F       18-38   R
sub-020 CY8     M       18-38   R

It has two participant_id columns. I'll need to exclude it for now I think.

EDIT: Looks like those subject ids are not even in the original file: https://openneuro.org/datasets/ds001971/versions/1.1.1/file-display/participants.tsv / mne-tools/mne-bids#1065

@larsoner larsoner marked this pull request as ready for review September 10, 2022 18:41
@hoechenberger hoechenberger enabled auto-merge (squash) September 10, 2022 18:55
@hoechenberger hoechenberger merged commit 3c6bf58 into mne-tools:main Sep 10, 2022
@larsoner larsoner deleted the fix-url branch September 12, 2022 15:51
@larsoner larsoner added the EOSS4 label Sep 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: Broken testing dataset add ds004107 to datasets
4 participants