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

Draft BEP031 #2

Open
wants to merge 30 commits into
base: master
Choose a base branch
from
Open

Draft BEP031 #2

wants to merge 30 commits into from

Conversation

mariehbourget
Copy link

@mariehbourget mariehbourget commented Sep 1, 2021

@jcohenadad
This is a draft PR of BEP031 for review and approbation on our side only.
Once it is completed, I will open a PR on the main BIDS repo.

I recommend using mkdocs to see the rendered version of the doc.

The two files to review are:
src/04-modality-specific-files/10-microscopy.md --> will appear under Modality specific files/Microscopy in the rendered doc.
src/schema/entities.yaml --> will appear under Appendix/Entities in the rendered doc.

Other files do not need review at this point.

In the final version (after community review), the tables and the file tree examples will be generated automatically via macros and specific yaml files. I was asked to do "manual" tables and examples to minimize the number of changed files and facilitate community review.

A few remaining points to address, details in comments in the 10-microscopy.md file.

  • If we have a paper, add the link in Citing BIDS
  • Complete BIDS-examples and add their names
  • Add other links to relevant dataset examples
    • update our ADS dataset
    • it would be great to have a "real" OMETIFF dataset.
  • Finalize "chunks" figure
  • StationName metadata description --> to check with maintainers
  • BodyPart metadata description --> to check with maintainers
  • Check matrix definition "Array of arrays of number" --> according to this "In JSON, array values must be of type string, number, object, array, boolean or null." and this "Matrices in JavaScript Object Notation (JSON) are represented in this list of lists.". So an array of arrays of numbers is allowed. --> to check with the maintainers.
  • Make sure JSON example is "plausible" (confirmed with NanoZoomer spec here).
  • Recommended participant data section may changed
  • Add "photo" example
  • Check with maintainers if make_filetree macro should be used before on only after community review.

Note: I converted this PR from "draft" to "ready for review" only because I had problems pushing new commits to the branch when in "draft". Not sure why.

@mariehbourget mariehbourget marked this pull request as ready for review September 1, 2021 23:24
@mariehbourget
Copy link
Author

Hi @jcohenadad,
If you wish to review, the files src/04-modality-specific-files/10-microscopy.md and src/schema/entities.yaml are complete and will be submitted as a PR to the main BIDS-repo next week.

Copy link
Member

@jcohenadad jcohenadad left a comment

Choose a reason for hiding this comment

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

LGTM!

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