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

[FIX] arrays of 3D coordinates MUST supply numeric values in x, y, z order #623

Merged
merged 1 commit into from
Oct 3, 2020

Conversation

sappelhoff
Copy link
Member

fixes #613

In this PR I add a clarification for all JSON fields that accept an object of arrays where the arrays are expected to contain 3D coordinates.

this is the clarification:

Each array MUST contain three numeric values corresponding to x, y, and z axis of the coordinate system in that exact order.


While doing this PR I stumbled over a small oddity: In MRI AnatomicalLandmarkCoordinates are expected to be in voxel units

Key:value pairs of any number of additional anatomical landmarks and their coordinates in voxel units (where first voxel has index 0,0,0) relative to the associated anatomical MRI

Whereas in MEG and EEG, it says:

Key:value pairs of the labels and 3-D digitized locations of anatomical landmarks, interpreted following the AnatomicalLandmarkCoordinateSystem

probably related to https://github.com/bids-standard/bids-examples/issues/190 ... this is not very well specified in BIDS currently and I lack the practical experience (use cases, broad overview of research practice) to improve the state of the art. cc @robertoostenveld with whom I have discussed about this in #520 (comment)

Copy link
Collaborator

@hoechenberger hoechenberger left a comment

Choose a reason for hiding this comment

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

Small nitpicks

@sappelhoff
Copy link
Member Author

Thanks @hoechenberger I'll have to adjust table pipes in a new commit. I'll wait a bit though, because I'll also have to fix "e.g." -> "for example", see #626

@@ -59,12 +59,12 @@ also encourage users to provide additional meta information extracted from the
manufacturer specific data files in the sidecar JSON file. Other relevant files
MAY be included alongside the original EEG data in `/sourcedata`.

Note the RecordingType, which depends on whether the data stream on disk
Copy link
Member Author

@sappelhoff sappelhoff Oct 2, 2020

Choose a reason for hiding this comment

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

@Remi-Gau I just rebased my changes on top of your "e.g." -> "for example" changes and noticed the trailing blank lines whitespace. Just as a hint for you: It may be nice (for any project you are doing) to configure your editor to automatically remove trailing whitespace.

Or is there a particular reason for having that?

@sappelhoff sappelhoff merged commit 113b724 into bids-standard:master Oct 3, 2020
@sappelhoff sappelhoff deleted the order branch October 3, 2020 12:17
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.

clarify order of coordinates in *...Coordinates JSON fields
3 participants