-
Notifications
You must be signed in to change notification settings - Fork 180
[ENH] Reorganise modality agnostic files section into multiple pages #2106
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
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2106 +/- ##
=======================================
Coverage 82.10% 82.10%
=======================================
Files 17 17
Lines 1531 1531
=======================================
Hits 1257 1257
Misses 274 274 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Hi ! |
Hi everyone. Is it okay to merge this PR or are there more updates that would be required? As a note, this is only a refactoring of existing info (no new and/or no remove text). Thanks! |
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 am a little uncertain about using "Data descriptions" here, since "Dataset" includes "Data" as well.
Especially since to a degree summaries like participants.tsv
are also "dataset level description": although they already were separated from "Dataset description" section, they were not explicitly sectioned under "Data" section.
It seems that what is common among all, but phenotype/
, those "Data description" files -- they are directly pertinent to data under sub-*
folders. phenotype/
, sure thing, also but it is under a separate folder. I wonder if we should
- also move out Phenotypic and assessment data into its own file/sub-section.
- rename "Data description" into "Data summary files" since all those sections are "{entity/concept} files" (
concept
is forscans
one) and thussrc/modality-agnostic-files/data-summary-files.md
- this would also better align with a potential bids-2 level enhancement of Replace "inheritance" with "summarization" principle bids-2-devel#65 and Formalize "life-cycle"/movement and placement of metadata bids-2-devel#85 .
Hi @yarikoptic thanks for the review, I changed the names of the "Data description" section and added a new one for Phenotypic and assessment data, as you suggested. |
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.
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.
LGTM, apart from the redirects.
Hey @yarikoptic @effigies are we good to merge with your two approvals or should we wait for more input? Sorry just not sure about the process here... Thanks! |
I think we can merge. Been over a week since significant changes, just touch-ups since then. |
but ideally we are still to add more redirects whenever someone (might be us) brings that mkdocs/mkdocs-redirects#73 over the finish line |
BEP028 BIDS-Prov is going to introduce a lot of contents into the Modality agnostic files section of the BIDS specification.
In order to prepare for these changes, this PR splits the contents of the
src/modality-agnostic-files.md
file into separate files in asrc/modality-agnostic-files/
directory:src/modality-agnostic-files/dataset-description.md
src/modality-agnostic-files/data-description.md
src/modality-agnostic-files/code.md