Submitting author: @jdkent (James Kent)
Repository: https://github.com/HBClab/NiBetaSeries
Version: v0.3.2
Editor: @arokem
Reviewer: @snastase
Archive: 10.5281/zenodo.3385339
Status badge code:
HTML: <a href="http://joss.theoj.org/papers/a0d2ec4e06309e9b1e21dc302c396290"><img src="http://joss.theoj.org/papers/a0d2ec4e06309e9b1e21dc302c396290/status.svg"></a>
Markdown: [](http://joss.theoj.org/papers/a0d2ec4e06309e9b1e21dc302c396290)
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) 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.)
@snastase, please carry out your review in this issue by updating the checklist below. If you cannot edit the checklist please:
The reviewer guidelines are available here: https://joss.theoj.org/about#reviewer_guidelines. Any questions/concerns please let @arokem know.
✨ Please try and complete your review in the next two weeks ✨
paper.md file include a list of authors with their affiliations?Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @snastase it looks like you're currently assigned as the reviewer for this paper :tada:.
:star: Important :star:
If you haven't already, you should seriously consider unsubscribing from GitHub notifications for this (https://github.com/openjournals/joss-reviews) repository. As a reviewer, you're probably currently watching this repository which means for GitHub's default behaviour you will receive notifications (emails) for all reviews 😿
To fix this do the following two things:


For a list of things I can do to help you, just type:
@whedon commands
Attempting PDF compilation. Reticulating splines etc...
@snastase : have you had a chance to take a look?
@arokem @jdkent Sorry for losing track of this! Working through it now...
Okay, I think I have a grip on this now—nice contribution and great to see things like this wrapped up into BIDS Apps! To summarize, this project aims to compute beta-series correlations on BIDS-compliant data preprocessed using fMRIPrep. I have some general comments and then a laundry list of smaller-scale suggestions. For the really nit-picky stuff, I can make a PR if you don't mind. Also, bear in mind that I'm more neuroscientist than software developer per se, so apologies if any of these comments are way off the mark!
_General comments:_
First of all, the PDF and introductory documentation (betaseries.html) could be made a little more clear and concise. For example, it wasn't immediately obvious to me what the actual output of the tool is... can I get the actual series of estimated betas? Or only the inter-ROI beta-series correlation matrices? It might be nice to simply get the beta series and forget the correlations (e.g., for a downstream MVPA analysis). Is it absolutely necessary to provide an atlas to define ROIs? If I don't provide an atlas, will it compute beta-series correlations between all voxels (computationally intensive). Basically what I'm trying to say here is that it wasn't obvious what to expect in terms of input–output (namely output), what moving parts are necessary, and how much flexibility there is. I could figure these things out by trial and error, but it seems useful to lay this out a bit more explicitly in the documentation.
I'm a little bit unsure about the Jupyter Notebook-style tutorial walkthrough in the "How to run" documentation. If I was planning to run this, I'd likely be running it from the Linux command line (maybe via a scheduler like Slurm on my server)—not invoking it via Python's subprocess. You jump through a bunch of hoops with Python just to download and modify the example data and only one cell of the tutorial actually runs nibs. I think this material is useful, particularly for users to see how to modify idiosyncratic OpenNeuro datasets, but I'm not sure there's enough focus on the nibs invocation. An alternative approach would be to upload the minimal dataset with all necessary modifications to figshare or something and download that in the tutorial. This would avoid spending so much of the tutorial on data manipulation and spend a few more cells describing the nibs command-line invocation and various options.
This brings up the point that, if the preprocessed BIDS derivatives (e.g., in *_events.tsv) are fairly standard, should we expect nibs be able to handle them internally? For example, you manually rename some columns and reassign "Yes/No" values to 1 and 0 to satisfy assumptions. Another approach to this would be to build in some optional arguments in the nibs CLI that allows the user to specify column names and acceptable value names (and map to numerical values if need be). For example, when I run nibs, I might have a command-line argument specifying that conditions is the column name indicating trial types and that it should have three possible conditions values (neutral, congruent, and incongruent), and another argument that specifying the correct column and mapping {'Yes': 1, 'No': 0}. I'm not necessarily saying this _should_ be the way things are, just offering this an alternative approach. I'm genuinely curious if this would be feasible and if it's better or worse in terms of software design.
_Specific comments and questions:_
pip install nibetaseries in a conda environment or something along these lines.Speaking of installation, if you're degenerate like me and still have a Python 2.7 installation on your machine, pip install nibetaseries will try to install this in 2.7 and wreck. Something like pip3 install nibetaseries or python3 -m pip install nibetaseries works more reliably. I would slightly expand the installation documentation page, specifying Python version, etc.
It might be worth pointing users to Binder (https://mybinder.org) for running the tutorial Jupyter Notebook interactively.
I understood that based on the atlas provided, beta series are computed per voxel then averaged across voxels within each ROI? As opposed to averaging time series across voxels then computing the beta series? Is there a reason (or reference) for taking this approach?
What are the recommendations for high-/low-pass filtering? Is there a precedent in the literature for any recommended values? In fact, the documentation mentions both low- and high-pass filtering, but I only see an option for supplying a low-pass filter in the usage documentation.
Some of the multiword command-line arguments use "_" and some use "-" ...I would just use underscore in e.g. --atlas-img for consistency with other arguments (e.g., --session_label, --hrf_model)
Is this backward-compatible with older-style BIDS derivatives from fMRIPrep? E.g., files with and without the "desc-".
One issue I've encountered with people running apps like fMRIPrep is confusion about the "work" directory; namely, whether it can be safely deleted, whether it should be deleted if re-running from scratch, etc. Would be good to make a note of this in the documentation.
In the documentation and PDF, I would make it a little more explicit that the "beta" is a colloquial term for the parameter estimates (or regression coefficients) in a GLM.
It should be made abundantly clear in the documentation that this is running the "LSS" version of the analysis. Are there future plans to allow for optionally running the "LSA" version?
The “How to run NiBetaSeries” section of the documentation unpacks strangely and doesn’t allow user to scroll down through headings; at first I thought the download links were the only thing there. Clicking on the subheadings in the table of contents, however, brings you to a separate page with the walkthrough. Is there a way to combine these such that the download links simply appear at the top of the same page as the walkthrough?
Under the "References" heading in the betaseries.html documentation, I would include the full reference text and DOI links.
Cite the paper for the OpenNeuro dataset you use in the tutorial documentation:
Verstynen, T. D. (2014). The organization and dynamics of corticostriatal pathways link the medial orbitofrontal cortex to future behavioral responses. _Journal of Neurophysiology, 112_(10), 2457–2469. https://doi.org/10.1152/jn.00221.2014
I would cite Abdulrahman & Henson (2015) in the PDF.
I would also cite the BIDS Apps paper in the PDF as this is a BIDS App:
Gorgolewski, K. J., Alfaro-Almagro, F., Auer, T., Bellec, P., Capotă, M., Chakravarty, M. M., ... & Poldrack, R. A. (2017). BIDS apps: Improving ease of use, accessibility, and reproducibility of neuroimaging data analysis methods. _PLOS Computational Biology, 13_(3), e1005209. https://doi.org/10.1371/journal.pcbi.1005209
In the PDF, update the fMRIPrep reference by Esteban et al. to the Nature Methods version:
Esteban, O., Markiewicz, C., Blair, R. W., Moodie, C., Isik, A. I., Erramuzpe, A., Kent, J. D., Goncalves, M., DuPre, E., Snyder, M., Oya, H., Ghosh, S., Wright, J., Durnez, J., Poldrack, R., & Gorgolewski, K. J. (2019). FMRIPrep: a robust preprocessing pipeline for functional MRI. _Nature Methods, 16_, 111–116.
Code coverage is only 70%... might be worth trying to increase this.
Thank you so much for the in-depth review @snastase! I will be working on addressing these comments via issues/pull requests this week.
👋 @jdkent - What's going on with this submission? Are you working on the comments? Or maybe you have worked on them already, and just need to tell us here?
Hi @danielskatz, I am working through the comments still. I should have more time to dedicate to the project this week and the next.
Thanks for the update.
@jdkent — Can you give us an update? If you're not close to done, please let me know of a time period to set a reminder for you.
Hi @jdkent, sorry for the repeated bugging, but any updates?
Hi Sorry, I was at a conference and I missed responding to the last comment. This is high in my queue, I'm putting in a good faith effort to finish by the end of this week.
@whedon remind @jdkent in 1 week
Reminder set for @jdkent in 1 week
:wave: @jdkent, please update us on how things are progressing here.
Update:
I have at least touched on every issue (except adding a binder link), which I am currently deriving a solution for. Thank you for the comments @snastase, I think the documentation is much improved now. @snastase, could you take another look at the joss issues to see if I adequately covered your comments?
the most up to date documentation should be published on RTD
@jdkent sorry, I'm trying to get to this! Very backed up right now. Hopefully by this weekend.
Hey @snastase : have you had a chance to take another look?
@jdkent sorry the delay—finally got to work back through this. I think this is greatly improved. I went through the documentation and made a PR with minor edits.
I'm satisfied with this but am providing a few additional comments:
Y = X_1\beta_1 + X_n\ne1\beta_n\ne1 + \epsilon,
Y = X_2\beta_2 + X_n\ne2\beta_n\ne2 + \epsilon, ...
Important not to conflate "resting-state", which is a task paradigm, with the correlation-based functional connectivity analyses typically run on resting-state data. I tried to make this a bit clearer in the documentation.
I confirmed that tutorial runs on Binder. For whatever reason, the link to Binder in on the Tutorials documentation points a kind of off-putting 404 error until the Binder session initializes. (Note also that the References are not rendering properly at the bottom of the Jupyter notebook).
Do you really want to introduce Generalized Linear Models in the PDF? Not sure this (and the discussion of e.g. non-normality) is really representative of how GLMs are typically used in fMRI. I would also not say the "peak of the gamma curve is determined by the beta coefficient".
I would probably the de-emphasize the importance of the "atlas parcellation" in the PDF. Using an atlas to compute inter-areal correlations is only one particular strategy. For example, you could also compute beta series per voxel and then use ICA to identify "networks" rather than using an a prior parcellation.
In general, I would do a final pass through the PDF and make sure it's concise as possible. For example, I would take out unrelated bits like "voxel is shortened form of volumetric pixel" to make the PDF as short and direct as possible.
I'm curious why you would compute beta series per voxel then average them within each ROI rather than the much less computationally-intensive approach of averaging the time series in each ROI then computing beta series—but this is not a critical issue.
@jdkent : how are things going? When do you anticipate finishing your current revisions?
I hope to have the current revisions done by Friday this week.
@arokem, I believe I have responded to all of @snastase's comments, and have either done revisions or provided reasons why the change is not happening yet, while leaving the issue open.
@whedon generate pdf
Great! @snastase : could you please confirm that everything has been addressed? I see that all the check-boxes are checked, which means that this is probably ready to go.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
Great! @snastase : could you please confirm that everything has been addressed? I see that all the check-boxes are checked, which means that this is probably ready to go.
@arokem Good to go on my end! @jdkent has addressed my comments.
Great! Thanks for all your work on this review, @snastase. I concur on my end.
At this point, @jdkent : could you please create an archive of your repository (e.g., using Zenodo) and then post the DOI here. When creating the archive, please make sure that:
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@arokem here is the doi: 10.5281/zenodo.3382330
I had some trouble with zenodo, but I think I have it all worked out, the page is here
I second that big thanks to @snastase! and thank you @arokem for keeping us moving along!
Hi @jdkent : I see that the Zenodo submission has two authors (you and Peer), but you are the sole author of the paper. That needs to be consistent. I believe that you should be able to edit the Zenodo submission metadata.
I remade the paper with Peer as a co-author, or were you referencing another document?
OK. I see that he approved that in https://github.com/HBClab/NiBetaSeries/pull/181.
@whedon set 10.5281/zenodo.3382330 as archive
OK. 10.5281/zenodo.3382330 is the archive.
@openjournals/joss-eics : I believe that this is now ready for your review
@whedon accept
Attempting dry run of processing paper acceptance...
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/939
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/939, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true
I see that the latest version tagged on the GitHub repo is v0.3.0, while here we have v0.2.3. Please confirm that we should change the version here to match the tag in the repo. Also, it is marked as a "Pre-release." Are you sure you want that? It generally means the release is unstable—if that were the case, you would not be publishing a paper about the software at this time.
Since I'm sending this back to you with release questions, might as well add minor edits: please capitalize Python and Matlab, use commas after "e.g." and "i.e." and make one final pass at conciseness. For example, you can delete "With the concept of beta under our belt" (excess clutter). The phrase "Neurohackademy is to thank" is grammatically awkward. I'm also a bit unsatisfied with your headings: Introduction, Background, Overview. It seems all headings in the paper could be shuffled around without harm to the narrative, which makes me think they are superfluous. Perhaps just one Background heading and one Software Overview? (We sometimes also see Statement of Need, given that this is one of the requested topics in our docs. Give it a thought.
Thanks for noting the pre-release check, that was a mistake.
Thank you for the feedback @labarba, I believe I've addressed your comments in the referenced pull request (HBClab/NiBetaSeries#185)
Note, I've merged in one pull request since the making the zenodo doi, with another on right on it's heels, so I could just do one more minor release and say v0.3.1 will be the JOSS release when merging HBClab/NiBetaSeries#185.
The alternative as I see it is to modify the v0.3.0 release, either way, I would like to change the version from v0.2.3.
Let me know if you have any preferences for modifying v0.3.0 or cutting a new patch release (v0.3.1) for the JOSS submission.
Thanks!
Feel free to increase the patch release, and report the final version number here, so we can add it.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
v0.3.2 Hi @labarba, I believe the repository is ready pending the points above.
@openjournals/joss-eics : is everything in order here?
I'm not on rotation, but could you now check that the issues I flagged were resolved (version, etc.), and update as needed with whedon?
☝️ @arokem
@whedon set version as v0.3.2
@whedon set archive as 10.5281/zenodo.3385339
Either I'm having some syntax errors, or @whedon is taking a nap.
@whedon set 10.5281/zenodo.3385339 as archive
OK. 10.5281/zenodo.3385339 is the archive.
@whedon set v0.3.2 as version
OK. v0.3.2 is the version.
@arokem ☝️
Thanks! 🤦♂
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@arfon the link to the archive in the PDF does not seem to work. Is this normal at this point?
@jdkent can you work on/check the following for your paper:
@whedon generate pdf to update your proof@arokem could you please verify the above once @jdkent has worked on them, and ping me when we can proceed to the next step? Thanks
@arfon the link to the archive in the PDF does not seem to work. Is this normal at this point?
The link to the archive works in the final proofs (when you do @whedon accept)
@Kevin-Mattheus-Moerman, thank you for your suggestions! For making the proof, is it alright if the paper on zenodo/GitHub tag is slightly different than the proof that is used in the journal?
I can email zenodo and ask them to update the repository and I can modify which commit v0.3.2 points to (but I cannot change the pypi release).
However, it would be easier if I could edit the JOSS paper on master and have the git tag and zenodo archive contain a slightly different JOSS paper.
@jdkent It is okay that the Zenodo archived paper is different from this one. The purpose of the archive is to capture the software. Ping me here when you've implemented the changes.
@arokem could you check this pull request to make sure I covered the above points? Thanks!
(also it should perhaps be: "Interdisciplinary Graduate Program in Neuroscience"?), should it perhaps be "Psychological and Brain Sciences" instead, like in your profile page?
I'm keeping the Interdisciplinary Graduate Program in Neuroscience as my affiliation since that is what my PhD will say when I graduate.
@whedon render pdf
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Oh - of course I don't see these changes in the PDF. They're on a separate branch.
Looks alright to me.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@Kevin-Mattheus-Moerman
From this commit:
Let me know if I've covered all your suggestions adequately.
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@jdkent thanks, this looks good now.
@whedon accept
Attempting dry run of processing paper acceptance...
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/966
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/966, then you can now move forward with accepting the submission by compiling again with the flag deposit=true e.g.
@whedon accept deposit=true
@whedon accept deposit=true
Doing it live! Attempting automated processing of paper acceptance...
🐦🐦🐦 👉 Tweet for this paper 👈 🐦🐦🐦
🚨🚨🚨 THIS IS NOT A DRILL, YOU HAVE JUST ACCEPTED A PAPER INTO JOSS! 🚨🚨🚨
Here's what you must now do:
Party like you just published a paper! 🎉🌈🦄💃👻🤘
Any issues? notify your editorial technical team...
@jdkent congratulations on your publication in JOSS. Thank you @arokem for editing this submission and thank you @snastase for reviewing this work. :tada:
:tada::tada::tada: Congratulations on your paper acceptance! :tada::tada::tada:
If you would like to include a link to your paper from your README use the following code snippets:
Markdown:
[](https://doi.org/10.21105/joss.01295)
HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01295">
<img src="https://joss.theoj.org/papers/10.21105/joss.01295/status.svg" alt="DOI badge" >
</a>
reStructuredText:
.. image:: https://joss.theoj.org/papers/10.21105/joss.01295/status.svg
:target: https://doi.org/10.21105/joss.01295
This is how it will look in your documentation:
We need your help!
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:
@snastase - thanks again for your review!
@labarba & @Kevin-Mattheus-Moerman - thank you for helping making the text clearer and answering my questions!
@arokem - thank you for your guidance during this process!
This was a great experience!