-
Notifications
You must be signed in to change notification settings - Fork 11
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
Consolidated PR for #109 to #112, #139, #135. #137, #138 #140
Conversation
This is a consolidated PR for the items that the HWG has discussed for updates to 8.3.0. Please review carefully for discussion. |
I don't see any mistakes in the units/values/etc sections from what we discussed. Are we still planning to update the allowedCharacters on value classes for this release? |
Yes -- haven't gotten to it yet. Attached is a summary of the changes --- as generated by the new schema comparison tool (Thx @IanCa). In the current PR, I just changed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 😊
@@ -487,13 +487,16 @@ Each term in this vocabulary has a human-readable description and may include ad | |||
**** Female <nowiki>[Biological sex of an individual with female sexual organs such ova.]</nowiki> | |||
**** Male <nowiki>[Biological sex of an individual with male sexual organs producing sperm.]</nowiki> | |||
**** Intersex <nowiki>[Having genitalia and/or secondary sexual characteristics of indeterminate sex.]</nowiki> | |||
**** Other-sex <nowiki>[A non-specific designation of sexual traits.]</nowiki> | |||
*** Ethnicity <nowiki>[Belong to a social group that has a common national or cultural tradition. Use with Label to avoid extension.]</nowiki> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldnt this refer to a population or culture within which the ethnicity references?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you make a specific wording suggestion. Ethnicity isn't in BIDS so we are not constrained by a particular definitions. Thx @smakeig
There seems to be something wrong validating deprecatedFrom. I'm looking into it...
Fixed by this PR, it just hasn't been added to main develop branch yet: |
As discussed with HWG -- this PR is being merged so that changes will be available in the prerelease schema viewer. Comments are still possible as the schema itself is not being released yet. |
I would like to add the different EEG artifact types to the main HED schema. Would you like me to open a pull request? Biological artifacts would include the following:
Non-biological artifacts would include the following:
|
I agree we should add artifacts, but lets have a HED Working Group discussion about how artifacts will evolve to include specific artifacts from other modalities. What are the artifact concerns for IEEG? Are there fMRI artifacts that would fall under these tags? |
No description provided.