Skip to content

Fix: modality refers to modality suffixes#47

Closed
charlesbmi wants to merge 1 commit into
childmindresearch:mainfrom
charlesbmi:fix/modality-suffix
Closed

Fix: modality refers to modality suffixes#47
charlesbmi wants to merge 1 commit into
childmindresearch:mainfrom
charlesbmi:fix/modality-suffix

Conversation

@charlesbmi
Copy link
Copy Markdown

Thanks for this helpful library!

Looking at :
https://bids-specification.readthedocs.io/en/stable/appendices/entities.html#mod

it looks like mod- is only used for defacing masks, which might not be of interest to many. bids-examples only has 1 dataset that uses it.

BIDS seems to use modality to mostly refer to datatypes (already included in this API) or modality suffixes

@charlesbmi
Copy link
Copy Markdown
Author

Actually, this is not quite right either:

from: https://bids.neuroimaging.io/standards/schema/schema-meta.html?h=modality#context

modality:       Modality of current file, for example, MRI

should this actually just backwards-look this up from the
https://bids.neuroimaging.io/getting_started/folders_and_files/folders.html#datatype ? i.e. datatype & supported-suffix specifies the modality?

@clane9
Copy link
Copy Markdown
Contributor

clane9 commented May 6, 2025

Thanks for the PR and your interest in the library @charlesbmi! You are right, good catch. The mod entity has basically nothing to do with modality 😅. And indeed, the suffix seems to be the best way to get the modality.

I'm closing the PR though because the BIDSTable interface has been removed (at least for now) in bids2table 2.0 (#48). I feel like it is kind of unnecessary at this point, now that the table schema itself has been simplified to just the dataset, path, and BIDS entities. Although, please feel free to open a new issue if you think some BIDSTable-like convenience interface to the underlying table would be useful to have.

@clane9 clane9 closed this May 6, 2025
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.

2 participants