-
Notifications
You must be signed in to change notification settings - Fork 60
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
add SAMRI, update BEP001 link #222
Conversation
- name: SAMRI | ||
url: https://github.com/IBT-FMI/SAMRI | ||
updated: 2021-11-08 | ||
data_types: small-animal MRI | ||
language: Python, shell | ||
documentation: https://doi.org/10.3389/fninf.2020.00005 | ||
GUI: false |
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.
@TheChymera this PR adds the SAMRI converter to the list of bids compatible data converters/tools (see https://bids.neuroimaging.io/benefits.html#mri-and-pet-converters).
I filled in the info as well as I could from a brief glance --> could you please (by looking at the other entries in this YML file) add information that I missed, and/or correct what I got wrong?
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.
@sappelhoff thank you! I could help out with this, but I can't really understand where/why the markdown link check fails :-/
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 fixed it by removing it :) ... was some link in an old blog post that nobody is going to miss.
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.
but your help would be appreciated by either approving of the changes ... or telling me which additional info I should include.
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.
The URL does still resolve... https://journals.sagepub.com/doi/10.1177/0271678X20905433. May have been a temporary failure?
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.
fair point, I added it back and changed the link to be a proper doi.org
link instead of the sagepub
link. Although it might just redirect there anyways.
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.
Failure may be related to tcort/markdown-link-check#109.
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.
@sappelhoff should I just write a comment here for what's to be added, or submit a PR to your branch? I think it might be easiest if I just submit a PR for additional info after this is merged.
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.
@TheChymera sounds good - looking forward to your follow-up PR to improve the information!
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.
#224 for the follow-up :)
closes #220
closes #221