Skip to content
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

Paper fixes for JOSS review #750

Merged
merged 3 commits into from
Apr 25, 2024
Merged

Conversation

britta-wstnr
Copy link

This PR is done within the reviewing process over at openjournals/joss-reviews#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:

  • the authors John T. Wodder and Todd Thompson have no ORCIDs attached. Please make sure that's on purpose.
  • 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.
    NIfTI Data Format has a funny year ("2003--") - please double check.
  • 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)

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?

Copy link
Member

@yarikoptic yarikoptic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I approve changes but would like to address other items listed in PR description before merging.

Thank you @britta-wstnr

@yarikoptic
Copy link
Member

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 #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 4583f60

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 954c159 .

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

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 26072be

  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
Copy link
Author

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
Copy link
Member

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.

@yarikoptic yarikoptic merged commit bd4c5dd into nipy:paper-joss Apr 25, 2024
1 check failed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants