-
Notifications
You must be signed in to change notification settings - Fork 7
[ENH] Document new dataset description file #351
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
✅ Deploy Preview for neurobagel-documentation ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
surchs
left a comment
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.
Thanks @alyssadai for writing the docs! Reads well and clear!
I have some high level suggestions that I think would make this easier to digest for a new user:
- Put the example of the file at the very top, right after a (brief) explainer. Shows the user what we're dealing with
- Have the "editable template" second. Gets the user going immediately and avoids having to sift through text that may or may not be relevant.
- Add a downloadable template file as well - parallel to the template text box. Lets user go direct to their text editor if wanted and avoids additional step of "create new text file first".
- Turn the BIDS / not-BIDS part into just an admonition. Avoids overlap and cognitive load for user to parse the difference between sections. Main message: "you may have this file already from BIDS, in this case just add the keys you don't have from the template as needed" - this should be easy because a BIDS dataset_description.json is valid for us (reverse not, but that's not a real worry because if they didn't have a dataset_description.json to begin with, then their BIDS dataset was never valid).
|
@surchs, thanks for your suggestions! I implemented them all, but there's one that probably needs another look:
So because JSON can be rendered directly in the browser, I don't actually think (or haven't found a way) to force a direct file download for a JSON file using either markdown attributes or HTML tags - for me at least, it always opens in a separate tab as plaintext. The only workaround I could find was to zip the file first, which I've done in the latest commit. Is that fine do you think? |
|
Hey @alyssadai
Interesting, I hadn't expected this. I'm not sure what to do here. First of all: great solution. But the reason I suggested the file is to make it easier on the user to use our template. Seems to me that unzipping a file is not a lot easier than either saving a JSON file from a browser tab or copy-pasting the template directly. So I'd say in order of preference for me:
|
surchs
left a comment
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.
Thanks @alyssadai, I think this now reads very clear and straightforward. I also like the position of the "I have a file from BIDS dataset" info box - I was spotting that exactly when I was wondering about that question (i.e. after reading the editable box). Very cool.
🧑🍳
Changes proposed in this pull request:
--dataset-descriptionargmkdocs-madlibsplugin (and required stylesheet) for templated dataset description code snippetsChecklist
Please leave checkboxes empty for PR reviewers
[ENH],[FIX],[REF],[TST],[CI],[MNT],[INF]) see our Contributing Guidelines for more info)Closes #XXXXmkdocs.ymlconfig