-
Notifications
You must be signed in to change notification settings - Fork 161
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
BEP 18 suggestions #398
BEP 18 suggestions #398
Conversation
@claramoreau9 I'm unable to request a review from you, but assuming you're the extension developer, I would appreciate your eyes. |
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.
all good for me except genetic_info.json
- why removing the multiple entries, I do want to show that multiple info can be entered ; and indeed it is often the case that different analytics are provided
Cyril, I'm not sure what you mean about removing multiple entries. I don't think I changed anything in that respect. Could you show how you would like it to read? |
it's me who read to quickly, you only changed to have to the next line ... sorry "AnalyticalApproach": ["Whole Genome Sequencing","SNP/CNV Genotypes"], I think it's better, showing multiple options -- is that correct to use the square brackets and list items? FYI: https://github.com/CPernet/bids-examples/tree/master/genetics_ukbb |
9394e86
to
c13f171
Compare
With example provided by Cyril Pernet.
c13f171
to
8401a14
Compare
are we good on that one now? anything i need to do |
When this is ready to merge, please squash or rebase, so we don't bother polluting the changelog.
Suggestions: