-
Notifications
You must be signed in to change notification settings - Fork 169
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
[ENH] Add Level objects to channels.json for motion #1591
Conversation
for more information, see https://pre-commit.ci
034dfc5
to
a0e2c18
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1591 +/- ##
=======================================
Coverage 87.83% 87.83%
=======================================
Files 16 16
Lines 1356 1356
=======================================
Hits 1191 1191
Misses 165 165 ☔ View full report in Codecov by Sentry. |
Following up on bids-standard#1591
Thank you, I agree with all of the proposed changes. Example data sets in this PR reflects them already (renamed ReferenceFrameDescription -> Description).
Do you plan to open this one or should I have a look into implementing this? |
That was done in #1603. |
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.
I think mentioning Description
is a good idea, but feel free to ignore if you disagree. Anyhow, +1 to merge
Co-authored-by: Stefan Appelhoff <[email protected]>
*_channels.json
for the description of coordinate systems #1524.This was initially comments on #1524, but it became a lot and easier to just make a proposal. Making pull requests against other people's branches often fails, so might as well just make an alternative PR.
The things worth noting:
"n/a"
is strictly worse than omitting a field. In one case I can test for presence.sidecar.reference_frame.Levels.<rf_name>
object. I just changed it to Description.