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] Rename channels.tsv column: orientation_component to component #1417

Merged
merged 9 commits into from
Feb 15, 2023
Merged

[FIX] Rename channels.tsv column: orientation_component to component #1417

merged 9 commits into from
Feb 15, 2023

Conversation

sjeung
Copy link
Collaborator

@sjeung sjeung commented Feb 13, 2023

Following the discussion on issue:

(closes #1393)

  • update orientation_component column in *_channels.tsv to component to cover non-orientation spatial channels and
  • wording change in motion channel description in NIRS spec.

accompanying validator PR:

@sjeung
Copy link
Collaborator Author

sjeung commented Feb 13, 2023

Tagging @sappelhoff, I think reviewers were autoassigned

@codecov
Copy link

codecov bot commented Feb 13, 2023

Codecov Report

Base: 88.65% // Head: 88.65% // No change to project coverage 👍

Coverage data is based on head (b0f6058) compared to base (134f532).
Patch has no changes to coverable lines.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1417   +/-   ##
=======================================
  Coverage   88.65%   88.65%           
=======================================
  Files          11       11           
  Lines        1084     1084           
=======================================
  Hits          961      961           
  Misses        123      123           

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

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.

Thanks @sjeung!

I'll check if we would also need to adjust the BIDS validator if these changes were to be merged.


I think you also need to edit src/schema/rules/checks/nirs.yaml --> there is an OrientationComponent in there as well.

A good trick to catch every specific word you want to edit is to use an editor like VSCode that allows you to open a full directory and then search all files in that directory. for example in VSCode that can be done with Ctrl+Shift+F.

If you don't have VSCode but want to search a GitHub repo, you can also navigate to the repo in your browser and then press dot (.) to open up a VSCode instance in your browser, and from there you can do the search trick.

See screenshot:

tagging @JuliusWelzel as this trick might be handy to know for you as well.

grafik

src/modality-specific-files/near-infrared-spectroscopy.md Outdated Show resolved Hide resolved
src/schema/objects/columns.yaml Show resolved Hide resolved
@sappelhoff sappelhoff changed the title [FIX] Rename orientation_component column to component [FIX] Rename channels.tsv column: orientation_component to component Feb 14, 2023
@sappelhoff sappelhoff added the consistency Spec is (potentially) inconsistent label Feb 14, 2023
@sappelhoff sappelhoff merged commit 68f91ca into bids-standard:master Feb 15, 2023
@sappelhoff
Copy link
Member

Thanks @sjeung and all involved reviewers.

@sjeung and @JuliusWelzel I hope you can now proceed with taking these adjustments to the spec, validator, and examples so that we'll soon be ready for the BEP029 MOTION community review 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consistency Spec is (potentially) inconsistent
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"orientation_component" in BIDS-NIRS conflicts with "component" in motion BEP
7 participants