-
-
Notifications
You must be signed in to change notification settings - Fork 37
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
[REVIEW]: HeuDiConv — flexible DICOM conversion into structured directory layouts #5839
Comments
Hello humans, I'm @editorialbot, a robot that can help you with some common editorial tasks. For a list of things I can do to help you, just type:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
|
Wordcount for |
|
Hello again! 👋
This is the review thread for the paper. All of our higher-level communications will happen here from now on, review comments and discussion can happen in the repository of the project (details below). 📓 Please read the "Reviewer instructions & questions" in the comment from our editorialbot (above). |
Review checklist for @marcelzwiersConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hi @JLefortBesnard, let me know if you have any questions or anything is unclear regarding the next steps! |
Ping @JLefortBesnard again, would you mind generating your checklist to see everything is in order? Thanks a lot! 🙏 |
Heudiconv is probably the best known and most used software tool for researchers who would like to do open science and organize their data according to the BIDS standard. The heudiconv paper is well written but brief and rather incomplete without the online read-the-docs documentation. I felt that after reading the paper I didn't get an idea about the architecture of Heudiconv, i.e. how the heuristics are used, what the main workflow is (the paper mentions "discovery, manual tuning, conversion" steps without properly explaining what they are. In the tutorials it is mentioned that you first may have to generate a About the paper, I have a few more comments: Figure 1 contains details about the data acquisition (k-space!) that have nothing to do with heudiconv. Also, the figure contains several post-heudiconv use cases, which are very general and uninformative about heudiconv. In the figure, heudiconv itself is just one black box! Altogether, I suggest to remove the figure or replace it with a flow-chart of heudiconv itself (the inside of the black box), or of it's workflow (or both). Open general checks:
Figure 2 from the paper contains original data (arguably) but I cannot access the data nor reproduce the figure. I looked into etelemetry but the data is stored on a private server. I also looked at the pypi data, but in order to access that I believe I would need a (payed) Google Cloud BigQuery account. I am fine with accepting these checks if the editor does not consider download metrics as original data (although it would be nice if the authors explain a bit more on how the data was generated) Open functionality checks: The paper makes many functional claims, e.g. that it can handle complex data structures, data acquisitions schemes, and much more. I could not verify all claims but did try to convert a (relatively simple) standard dataset using the Open documentation checks:
I have already touched upon these checks in my general comment. Several examples (if third party tutorials count as examples) can be found on the read-the-docs website, but not in the paper. I would have liked to see a simple example in the paper (not in detail, but to get an idea what of what is involved and what the user would need to do). The same is true for the core functionality, it is described at the website but the paper basically only states "that decision making is implemented in HeuDiConv heuristics" and that heudiconv can operate in BIDS mode with a Open software paper checks:
Importantly, this is missing entirely. The limitations of heudiconv are not discussed (I think it should at least be mentioned that you have to be able to program Python, get to know the heudiconv API + that you need to have some understanding of pulse sequence parameters), neither is there discussion how heudiconv's advantages and disadvantages compare to those of other tools in the field. |
Hi @marcelzwiers - *one reason for this is that API documentation in papers often becomes obsolete after a few years, while the docs are a "living" thing that changes alongside the code. |
@marcelzwiers thank you for the review! And thank you @britta-wstnr for a valuable comment: we will take it into account while responding to the review(s), which we hope to do within a week. |
Review checklist for @JLefortBesnardConflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
|
Hello, I am really sorry for the delay, I didn't realize the reviewing had started, I'll get started and provide my feedback quickly. Thank you @britta-wstnr for the reminder email 🙂 |
Hello, Many great tutorials are available, I followed two of them and found them quite clear. In fact, the tutorials' quality accentuates the disparity with the "read-the-docs" even more. Though it is sufficiently clear to individuals who have prior experience in converting Dicom to Bids using Python (that is why I did check the 'functionality documentation' box), I feel that the documentation is perhaps not designed in the most user-friendly manner for newcomers, and thus does not provide enough guidance to make the usage more intuitive. Striving in this direction might be beneficial in promoting the use of this tool. I find the paper well-crafted and providing enough information for a solid understanding of the software. I appreciate Figure 1, which effectively illustrates the software's functionality. However, I have reservations about Figure 2, which, in my opinion, may not be relevant to the user since the information can be adequately summarized in the text. Instead, including a second figure that depicts the inner workings of the software, as the first reviewer suggested, would be more engaging. I also agree with the first reviewer about the missing limitations of HeuDiConv and the missing comparison with other similar tools (or ways) to convert DICOM into Bids (that is why I did not check the 'state of the field' box). I did not test all functional claims made in the paper ("complex organization scenario") thus cannot confirm them. Overall, I think this paper belongs in JOSS. This tool is poised to make a significant impact on the neuroimaging community, and I will use it too. |
Thank you @JLefortBesnard for the review! Would you mind sharing which particular 2 tutorials you found "quite clear"? |
Sure, I enjoyed the tutorials from James Kent and the one from Stanford |
Hi @yarikoptic - just checking in if everything is going alright here? 🙂 Or if there is anything you'd need help with from the editorial side? |
Hi @britta-wstnr , thanks for checking in! Sorry for the delay - were busy with SfN booth fun and now all involved are fighting the COVID after that fun ;) I hope we would get back to this by early next week. |
No worries at all @yarikoptic - I just wanted to make sure there are no open questions. Get well soon! 🍵 |
@yarikoptic I hope you are feeling better. Just checking in here - also to say that I will be out of office for winter break soon, so I might be slow to respond for a while. ❄️ ⛄ |
@editorialbot accept |
|
Ensure proper citation by uploading a plain text CITATION.cff file to the default branch of your repository. If using GitHub, a Cite this repository menu will appear in the About section, containing both APA and BibTeX formats. When exported to Zotero using a browser plugin, Zotero will automatically create an entry using the information contained in the .cff file. You can copy the contents for your CITATION.cff file here: CITATION.cff
If the repository is not hosted on GitHub, a .cff file can still be uploaded to set your preferred citation. Users will be able to manually copy and paste the citation. |
🐘🐘🐘 👉 Toot for this paper 👈 🐘🐘🐘 |
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨 Here's what you must now do:
Any issues? Notify your editorial technical team... |
I noticed that the automatically generated crossref file has some errors in how first names/last names were parsed. I'm not sure if it matters but I just wanted to point it out. https://github.com/openjournals/joss-papers/blob/master/joss.05839/10.21105.joss.05839.crossref.xml
|
If the names are described in the .md file as explained in https://joss.readthedocs.io/en/latest/paper.html#names, this shouldn't happen. If they aren't, they could be fixed, and the paper could be reaccepted to fix this. |
Thanks @danielskatz. We have fixed the .md file in nipy/heudiconv#772. |
the track chair, @Kevin-Mattheus-Moerman, should next do a reaccept to update the metadata deposited with Crossref. |
@editorialbot reaccept |
|
Element surname: [facet 'minLength'] The value has a length of '0'; this underruns the allowed minimum length of '1'.
Element surname: [facet 'pattern'] The value '' is not accepted by the pattern '[^\d\?]*[^\?\s]+[^\d]*'. |
@openjournals/dev @danielskatz do you know how to address this error? |
@Kevin-Mattheus-Moerman - I see two problems in the metadata, thought neither matches the error messages perhaps @xuanxu will have thoughts as well? |
Thanks @danielskatz. @yarikoptic Perhaps you can work on some of the points Dan mentioned so we can test that while we wait on @xuanxu to shed more light on this. |
big thanks @xuaxu! merged @editorialbot generate pdf |
@editorialbot reaccept |
|
🌈 Paper updated! New PDF and metadata files 👉 openjournals/joss-papers#5586 |
@yarikoptic can you confirm all looks good now? Thanks! |
Looks great to me, Seems to look good for @mvdoc 's name: <person_name sequence="additional"
contributor_role="author">
<given_name>Matteo</given_name>
<surname>Visconti di Oleggio Castello</surname>
<ORCID>https://orcid.org/0000-0001-7931-5272</ORCID>
</person_name> Huge Thanks EVERYONE who was with us on this journey! |
Congratulations @yarikoptic and team! |
Congratulations @yarikoptic Thanks for editing @britta-wstnr And a special thank you to the reviewers: @marcelzwiers, @JLefortBesnard !! |
🎉🎉🎉 Congratulations on your paper acceptance! 🎉🎉🎉 If you would like to include a link to your paper from your README use the following code snippets:
This is how it will look in your documentation: We need your help! The Journal of Open Source Software is a community-run journal and relies upon volunteer effort. If you'd like to support us please consider doing either one (or both) of the the following:
|
Submitting author: @yarikoptic (Yaroslav Halchenko)
Repository: https://github.com/nipy/heudiconv/
Branch with paper.md (empty if default branch): paper-joss
Version: v1.1.6
Editor: @britta-wstnr
Reviewers: @marcelzwiers, @JLefortBesnard
Archive: 10.5281/zenodo.11497270
Status
Status badge code:
Reviewers and authors:
Please avoid lengthy details of difficulties in the review thread. Instead, please create a new issue in the target repository and link to those issues (especially acceptance-blockers) by leaving comments in the review thread below. (For completists: if the target issue tracker is also on GitHub, linking the review thread in the issue or vice versa will create corresponding breadcrumb trails in the link target.)
Reviewer instructions & questions
@marcelzwiers & @JLefortBesnard, your review will be checklist based. Each of you will have a separate checklist that you should update when carrying out your review.
First of all you need to run this command in a separate comment to create the checklist:
The reviewer guidelines are available here: https://joss.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @britta-wstnr know.
✨ Please start on your review when you are able, and be sure to complete your review in the next six weeks, at the very latest ✨
Checklists
📝 Checklist for @marcelzwiers
📝 Checklist for @JLefortBesnard
The text was updated successfully, but these errors were encountered: