search

nipy / heudiconv

Flexible DICOM conversion into structured directory layouts
https://heudiconv.readthedocs.io
Other
232 stars 125 forks source link

Paper fixes for JOSS review #750

Closed britta-wstnr closed 4 months ago

britta-wstnr commented 4 months ago

This PR is done within the reviewing process over at https://github.com/openjournals/joss-reviews/issues/5839

@yarikoptic, I fixed some small things in the paper - please check if you agree that this might help readability.

Other things I came across:

And then there is three sentences that maybe could be made clearer (at least I stumbled when reading them):

  1. HeuDiConv has been developed to implement logic commonly used across labs (grouping DICOMs, extracting metadata, converting individual sequences, populating standard BIDS files, etc.) while allowing individual groups to customize how files should be organized and named while driving custom decisions through the conventions and desires of those individual groups. Maybe the sentence could be re-written (or split) to prevent the repetition of "while"?
  2. HeuDiConv, if instructed to operate in BIDS mode (--bids flag) with a heuristic providing base naming instructions, and helpers to organize the files in the hierarchy defined by the BIDS standard. I think this sentence is missing a main sentence part?
  3. Figure 1 caption: For more advanced usage at institutions with dedicated infrastructure, HeuDiConv can operate on an additional third machine, interfacing between the depicted two, and dedicated to data repositing, versioning, and backup. Maybe it could be made clearer which of the machines is dedicated to data reposition?
yarikoptic commented 4 months ago

Thank you for taking time to provide feedback to improve the paper.

@yarikoptic, I fixed some small things in the paper - please check if you agree that this might help readability.

I approved

  • the authors John T. Wodder and Todd Thompson have no ORCIDs attached. Please make sure that's on purpose.

AFAIK Wodder doesn't have ORCID, so intended. For Todd -- inquired in https://github.com/nipy/heudiconv/issues/754 and will give him a few days to reply.

  • please check the following two citations: Gorgolewski et al: due to different initials in the bibliography, the intitials occur in the reference in the text. Please check if that is correct/on purpose.

I only found him missing middle initial J. in one reference -- fixed for that in 4583f60e4eea32c1483391aa0eb417fbcf8f3719

NIfTI Data Format has a funny year ("2003--") - please double check.

well -- it is when the project started, then in subsequent years there were refinements etc, and it never "finished". So we kept the date open: limiting to 2003 (inception) would be somewhat misleading, specifying 2004 when poster appeared would also be incomplete.

  • Is the citation of Poldrack et al. 2004 at the right place? Should it be moved to after "one of the challenges"? (State of the field, first sentence, line 84)

IMHO it doesn't matter really, but if you like we could move. Done in 954c159f21931e69273cbdf71f9c1af2bf496bfe .

But also that publication is now published, so replaced arxiv citation with full fledged publication in 0ee95853201a99c67efdb0420e38b3c7c597283a

And then there is three sentences that maybe could be made clearer (at least I stumbled when reading them):

  1. HeuDiConv has been developed to implement logic commonly used across labs (grouping DICOMs, extracting metadata, converting individual sequences, populating standard BIDS files, etc.) while allowing individual groups to customize how files should be organized and named while driving custom decisions through the conventions and desires of those individual groups. Maybe the sentence could be re-written (or split) to prevent the repetition of "while"?

ok, split into two sentences, and pushed to this branch for review

  1. HeuDiConv, if instructed to operate in BIDS mode (--bids flag) with a heuristic providing base naming instructions, and helpers to organize the files in the hierarchy defined by the BIDS standard. I think this sentence is missing a main sentence part?

indeed! thanks for spotting. proposed fix pushed in this branch along with prior change in 26072be2d4c82c6f3e1e2ee5f0a4b07a7a192432

  1. Figure 1 caption: For more advanced usage at institutions with dedicated infrastructure, HeuDiConv can operate on an additional third machine, interfacing between the depicted two, and dedicated to data repositing, versioning, and backup. Maybe it could be made clearer which of the machines is dedicated to data reposition?

Well -- the Figure caption went into describing a hypothetical setup with the 3rd machine which is not depicted on the Figure. So in the 2-machine solution there is no such 3rd machine. If you really dislike this sentence which is not per describing the Figure content, we can just remove the sentence. Please advise.

britta-wstnr commented 4 months ago

Hi @yarikoptic, looks all good to me. Regarding the figure caption: ah, I understand this better now. I think the sentence is still a bit hard to parse, I'm making a suggestion below. But also fine to keep as is, up to you! 🙂

For more advanced usage at institutions with dedicated infrastructure, HeuDiConv can operate on an additional third machine, which then interfaces between the depicted two machines and is dedicated to data repositing, versioning, and backup.

yarikoptic commented 4 months ago

Thank you -- took your version. I think now we can proceed with this PR.

I will leave it up to you to decide on either to wait on

IMHO it is ok to proceed as is.