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.

Sign up for GitHub

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

Jump to bottom

[ENH] Define "modality" and clarify "data type" #592

Merged
merged 2 commits into from
Sep 16, 2020
Merged

[ENH] Define "modality" and clarify "data type" #592

merged 2 commits into from
Sep 16, 2020

Conversation

effigies
Copy link
Collaborator

@effigies effigies commented Sep 1, 2020 •
edited
Loading

Closes #584.
Closes #586.

Reworded slightly from initial suggestion.

@effigies effigies mentioned this pull request Sep 1, 2020
Closed
hoechenberger
hoechenberger approved these changes Sep 1, 2020
View reviewed changes
src/02-common-principles.md Show resolved Hide resolved
@tsalo
Copy link
Member

tsalo commented Sep 1, 2020 •
edited
Loading

WDYT of also slightly updating the definition of "data types" to clarify that they're reflected in folder names?

@tsalo tsalo mentioned this pull request Sep 2, 2020
Open
@effigies
Copy link
Collaborator Author

That one was actually a bit harder to word, IMO. Let me know what you think.

tsalo
tsalo approved these changes Sep 10, 2020
View reviewed changes
Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

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

I really like the updated "Data type" definition. I think this will really help with the confusion about terminology.

@effigies effigies changed the title ENH: Define "modality" ENH: Define "modality" and clarify "data type" Sep 10, 2020
@sappelhoff sappelhoff changed the title ENH: Define "modality" and clarify "data type" [ENH] Define "modality" and clarify "data type" Sep 11, 2020
@tsalo tsalo added this to the 1.4.1 milestone Sep 14, 2020
@effigies effigies mentioned this pull request Sep 15, 2020
Closed
@tsalo
Copy link
Member

tsalo commented Sep 15, 2020

Unless anyone raises any concerns about it, this PR should be good to merge tomorrow (5 days after second approval).

@sappelhoff
Copy link
Member

Unless anyone raises any concerns about it, this PR should be good to merge tomorrow (5 days after second approval).

I agree that it's good to be merged, it's a nice clarification.

@tsalo tsalo merged commit ce27309 into bids-standard:master Sep 16, 2020
@effigies effigies deleted the enh/modality branch September 16, 2020 17:17
bids-maintenance added a commit that referenced this pull request Sep 16, 2020
@bids-maintenance
[DOC] Auto-generate changelog entry for PR #592
c27f613
@yarikoptic yarikoptic mentioned this pull request Oct 8, 2023
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@agramfort agramfort agramfort approved these changes

@hoechenberger hoechenberger hoechenberger approved these changes

@tsalo tsalo tsalo approved these changes

@sappelhoff sappelhoff sappelhoff approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
1.4.1
Development

Successfully merging this pull request may close these issues.

Define "modality" in Common principles What to call modality directory vs the modality entity?
5 participants
@effigies @tsalo @sappelhoff @agramfort @hoechenberger