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

MINOR: Replace "units" with "unit" in channels.tsv #15

Open
tsalo opened this issue Aug 3, 2020 · 4 comments
Open

MINOR: Replace "units" with "unit" in channels.tsv #15

tsalo opened this issue Aug 3, 2020 · 4 comments
Labels
consistency Aspect requiring special treatment/logic outside of generic common principles impact: low Estimated low impact change metadata Changes to metadata fields/files.

Comments

@tsalo
Copy link
Member

tsalo commented Aug 3, 2020

All other column names are specified in singular (e.g., type, description, …) and it is not logical to have a unitS column.

Original authors: @sappelhoff

@tsalo tsalo added impact: low Estimated low impact change metadata Changes to metadata fields/files. labels Aug 3, 2020
@yarikoptic
Copy link
Contributor

we also use unit in schema. uniformity, adding 👍 and adding to project.

@yarikoptic yarikoptic moved this to Todo in BIDS 2.0 Apr 12, 2024
@yarikoptic yarikoptic added the consistency Aspect requiring special treatment/logic outside of generic common principles label Apr 17, 2024
@yarikoptic
Copy link
Contributor

announcement: since there is few more plural columns I generalized this issue into

@yarikoptic
Copy link
Contributor

it is also other units__ (for other modalities)

❯ git grep units__
src/schema/objects/columns.yaml:units__nirs:
src/schema/objects/columns.yaml:units__motion:
src/schema/rules/tabular_data/motion.yaml:    - units__motion
src/schema/rules/tabular_data/motion.yaml:    units__motion: required
src/schema/rules/tabular_data/nirs.yaml:    - units__nirs
src/schema/rules/tabular_data/nirs.yaml:    units__nirs: required

not only _channels.

There is also xyzt_units -- but it is a metadata (not .tsv column) field is intended to correspond to nifti header so I will leave it alone

❯ git grep xyzt_units
src/schema/meta/context.yaml:        xyzt_units:
src/schema/objects/metadata.yaml:    for units stored in 'xyzt_units' field) in the NIfTI header.
src/schema/rules/checks/nifti.yaml:    - nifti_header.xyzt_units.xyz != 'unknown'
src/schema/rules/checks/nifti.yaml:    - nifti_header.dim[0] < 4 || nifti_header.xyzt_units.t != 'unknown'

@yarikoptic
Copy link
Contributor

but then conundrum is that we do have "Units" as the metadata sidecar field (and additional ones like

❯ grep '^[^ ]*Units': src/schema/objects/metadata.yaml
AnatomicalLandmarkCoordinateUnits:
DigitizedHeadPointsCoordinateUnits:
EEGCoordinateUnits:
FiducialsCoordinateUnits:
HeadCoilCoordinateUnits:
InfusionSpeedUnits:
InjectedMassPerWeightUnits:
InjectedMassUnits:
InjectedRadioactivityUnits:
MEGCoordinateUnits:
MolarActivityUnits:
NIRSCoordinateUnits:
PharmaceuticalDoseUnits:
PixelSizeUnits:
ReconMethodParameterUnits:
SpecificRadioactivityUnits:
TracerMolecularWeightUnits:
Units:
iEEGCoordinateUnits:

So we might harmonize for consistency in .tsv header fields but break consistency with "Units" in sidecar... needs thinking!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
consistency Aspect requiring special treatment/logic outside of generic common principles impact: low Estimated low impact change metadata Changes to metadata fields/files.
Projects
Status: In Progress
Development

No branches or pull requests

2 participants