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

Updated examples as requested from specification PR #970 #302

Merged
merged 3 commits into from
Jan 15, 2022
Merged

Updated examples as requested from specification PR #970 #302

merged 3 commits into from
Jan 15, 2022

Conversation

VisLab
Copy link
Member

@VisLab VisLab commented Jan 13, 2022

This PR is done in conjunction with a rewriting of the 03 HED.md appendix in the BIDS specification
which corresponds to PR: bids-standard/bids-specification#970.

The following changes were made:

  1. The eeg_hed_small was renamed eeg_ds003654s_hed and updated to better correspond to the version of ds003654 on openNeuro.
  2. Additional copies eeg_ds003654s_hed_inheritance and eeg_ds003654s_hed_longform were added. These datasets are modifications of eeg_ds003654s_hed designed to demonstrate different forms of tag organization for testing and illustration.

Review requested: @sappelhoff @dorahermes @Remi-Gau

Copy link
Member

@sappelhoff sappelhoff left a comment

Choose a reason for hiding this comment

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

+1, it's good to have these examples (both for testing, and to point users to them). I think now the only thing we are missing is an HED example where an HED column in events.tsv is used, right? I know that feature is not really how HED should mainly be used ... but if it's a feature, it might as well be tested.

Thanks for editing the README, it's all very clear to me.

You need to remove the .idea/ folder that you accidentally (?) commited.

CI failures seem to be due to microscopy, which is not yet in "stable" validator

@VisLab
Copy link
Member Author

VisLab commented Jan 14, 2022

Thanks for spotting the .idea. Yes, the failures are due to microscopy and not these changes.

We'll add an example of the HED column in a future PR. There are a couple of other test cases regarding definition expansion that will also need to be added in a future PR.

I'd like to get this PR through with the HED.md update PR, so that we can move forward on the library schema update.

@sappelhoff sappelhoff merged commit f87c611 into bids-standard:master Jan 15, 2022
@sappelhoff
Copy link
Member

Merged- thanks @VisLab

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.

2 participants