Submitting author: @andrewthomasjones (Andrew Jones)
Repository: https://github.com/andrewthomasjones/BoltzMM
Version: 0.1.3
Editor: @yochannah
Reviewer: @ja-thomas, @schalkdaniel
Archive: 10.5281/zenodo.2563411
Status badge code:
HTML: <a href="http://joss.theoj.org/papers/23eb189a5e0bdd2b51f668621abcc75a"><img src="http://joss.theoj.org/papers/23eb189a5e0bdd2b51f668621abcc75a/status.svg"></a>
Markdown: [](http://joss.theoj.org/papers/23eb189a5e0bdd2b51f668621abcc75a)
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.)
@ja-thomas & @schalkdaniel, 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 @yochannah know.
β¨ Please try and complete your review in the next two weeks β¨
paper.md
file include a list of authors with their affiliations?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. @ja-thomas, 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...
Major comments:
The second author of the paper "Jessica J. Bagnall" does not seem to have commits to the repository. It might be a good idea to specify the contributions of every author in the README.
The tests cannot be reproduced with the instructions in the README
5 out of 11 tests fail when running locally, after calling devtools::document()
and devtools::test()
, all with XXX has changed from known value recorded in XXX
. (See https://github.com/andrewthomasjones/BoltzMM/issues/3)
Minor comments:
T
for TRUE
is rather bad practice in R since it can be easily overwritten as a variable. (As done in the installation instructions) knn.impute
produces warnings in the examplecitation("BoltzMM")
should be given in the READMEHi @ja-thomas, thank you for taking the time to review our paper! We very much appreciate your time and effort! π
I will reply to some of your comments now, and @andrewthomasjones will reply to the others at a later stage.
The second author of the paper "Jessica J. Bagnall" does not seem to have commits to the repository. It might be a good idea to specify the contributions of every author in the README.
Unfortunately, Jessica is not actively on GitHub (we will persuade her, yet!) and has therefore made most of her contributions via commits by @andrewthomasjones and I. She has made numerous contributions to the paper via the documentation of the package, the senate
data set that she had supplied and analysed from her paper https://hal.archives-ouvertes.fr/hal-01927188v1. As well as with identifying and helping to fix various bugs that were in previous versions of the code.
We have now included an Authorship statement section in the README in order to highlight the contributions of each of the three co-authors of the package.
knn.impute produces warnings in the example
Unfortunately, this cannot be help. No matter how the data is processed or preprocessed before conducting imputation, NA
s will be created since there is presence of missingness. Thus, a warning MUST be signalled at some part in time in the workflow. Since it is a little bit unsightly to have warning messages in the README, we have opted to use a suppressWarnings()
command in order to make the output more pleasing to the eye.
A reference to the paper given the theoretical framework for this method would be nice. (Or the JOSS paper, since it contains/references the required backround).
This was a great idea! We have now included a Technical references section in the README to address this comment.
There is no citation information in the README. At least the same information as
citation("BoltzMM")
should be given in the README
Again, great idea! We have now included a Reference to package section in the README that contains exactly the output of the command citation("BoltzMM")
as you have suggested.
Thanks again for your time, @ja-thomas. It is very much appreciated.
great, thanks for the quick reply and fix.
I guess the only point open from my side are the issues with the tests discussed here https://github.com/andrewthomasjones/BoltzMM/issues/3. When this is resolved everything is fine from my side.
all done from my side :+1:
Okay, looking good! @ja-thomas and @schalkdaniel thanks for reviewing. π
@schalkdaniel - You still have a couple of checkboxes unchecked. When you get a moment could you let us know if your concerns are resolved, or if not what needs to be done? π
All done from my side too. Everything looks great. π
@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
hmm, this is strange - these DOIs all seem to resolve correctly when I click on them.
I note there is a space as the last character for some of the DOIs which maybe we should fix, but that's only present on three so presumably can't be affecting them all!
@andrewthomasjones - it might be a good idea to remove those spaces from https://github.com/andrewthomasjones/BoltzMM/blob/master/paper.bib just in case?
@arfon / @openjournals/joss-eics - any idea what has whedon grumpy - something I missed or misunderstood maybe?
Hi @yochannah, spaces have now been removed. I also clicked on all of the links and they all seem to correctly direct to the papers. So π€·ββοΈ? Thank you for you editorial work! π
@yochannah - I've tried fixing the whitespace issue in https://github.com/openjournals/whedon-api/commit/c1d4e5e5c0bdb343207bf19e2dee2e2e8bd7304b.
More generally though, I think the issue is these should be doi
fields, not urls
, i.e. https://github.com/andrewthomasjones/BoltzMM/blob/master/paper.bib#L28 should be:
doi = {https://doi.org/10.1007/978-1-4614-6868-4}
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Hi @arfon and @yochannah, I've changed everything that should be a DOI from URL to DOI. There are a couple of items left that do not have DOIs, so I left them as URL. I hope that this resolves the issue! π
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@arfon , it doesn't look like a DOI exists anywhere on the internet for that last paper, unfortunately. π
@arfon , it doesn't look like a DOI exists anywhere on the internet for that last paper, unfortunately.
That's OK. These are only meant to be suggestions - it's fine to have references without DOIs if there isn't one.
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
@arfon thanks for clarifying what the issue was and @hiendn thank you for resolving the issues! :+1:
@hiendn @andrewthomasjones - at this point could you make an archive of the reviewed software in Zenodo/figshare/other service and update this thread with the DOI of the archive? It's probably worth giving the manuscript a final proofread if you feel you need to, as well. π
Hi @yochannah! Thank you for handling the editorial process. the Zenodo DOI/link is http://doi.org/10.5281/zenodo.2563411. And I have given the paper a final proof read and it is all good to go! π
π @hiendn β Could you edit the title in Zenodo so it matches the title of the JOSS paper? Thanks!
@whedon set10.5281/zenodo.2563411 as archive
@whedon set 10.5281/zenodo.2563411 as archive
OK. 10.5281/zenodo.2563411 is the archive.
Hi @labarba, the Zenodo archive name has been changed to match the paper title π.
@whedon accept
Attempting dry run of processing paper acceptance...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/491
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/491, then you can now move forward with accepting the submission by compiling again with the flag deposit=true
e.g.
@whedon accept deposit=true
@hiendn β I'm going to have a few more editorial fixes to request.
ΒΆ1
that is able densely represent >> is able _to_
when d is large and sample size n >> what about the sample size? (missing a verb and parallel construction)
In general, I find the whole first page really jargon-heavy and I wonder if you would consider editing this to be more readable. E.g., what is "latent variable construction"? Do you mean the same by "complex form" and "elaborate form" And what _do_ you mean by that? Jus that it's hard to implement? Or is it really "intractable"? Because then you talk about a "tractable simplification" ... so many big words. Are they all necessary for the JOSS audience to plough through, to finally learn what the software does on page 2?
Many of your DOIs are not resolving because you have a full URL instead of the DOI in the BibTeX field, so the link appears broken. Please check.
[EDIT] To clarify, the links come out as https://doi.org/https://doi.org/10.1016/S0364-0213(85)80012-4
which don't resolve.
Page 2:
All of the functions that have been comprehensively documented >> delete "that"
Furthermore, the analysis of data arising from⦠>> Unclear how this sentence is connected with the previous and the need for "Furthermore"
complimentary >> complementary (meaning: flattering versus supportive)
It's not necessary to include the GitHub repo address or the archive address in the text, because they will appear linked in the page-1 margin of the formatted paper, and will also be linked on the paper web page. Please remove.
Overall:
I suggest a pass of ruthless editing for clarity and conciseness. For example, how does one know the difference between "comprehensively documented" and "documented," between a report that "describes" versus "clearly describes"? What is "particularly computationally intensive" versus just "computationally intensive"? When is a package "sufficiently suitable" versus "suitable"?
What I'm trying to say, as an editor, is that the paper is too long and a slog to read, and I hope you can give it a bit of a clean.
Also, note the Author Guideline that says to "include in the paper some sentences that would explain the software functionality and domain of use to a non-specialist reader."
@whedon check references
Attempting to check references...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
@whedon generate pdf
Attempting PDF compilation. Reticulating splines etc...
Hi @labarba, thank you for the very comprehensive technical editing of the paper! π Your careful reading and many suggestions have been incredibly helpful in my reorganisation of the paper to be more suitable for the lay audience of the journal. Please find my responses to your comments, below. I hope that I have addressed all of your concerns . π
**@hiendn β I'm going to have a few more editorial fixes to request.
ΒΆ1
that is able densely represent >> is able _to_**
---In light of your comments below, regarding jargon, I have chosen to replace the word densely represent, with 'closely approximate', which I feel can be interpreted at face value by a lay reader, without the need for a dive into approximation theoretic nomenclature. The grammar of the sentence has also been fixed in order to comply with the new phraseology.
when d is large and sample size n >> what about the sample size? (missing a verb and parallel construction)
---Thanks for pointing out the typo, I have now change it to 'when both dimension $d$ and sample size $n$ are large.'
In general, I find the whole first page really jargon-heavy and I wonder if you would consider editing this to be more readable.
---Thank you for this comment. I suppose that I am new to writing more lay focused papers and so a lot of my technical writing habits come through. In what follows, I endeavour to choose better words that are meaningful to a lay audience, but do not detract from the technical meaning, either.
E.g., what is "latent variable construction"?
---'Latent variable construction' is now replaced by 'requirement for hidden (or unobserved) variables'.
Do you mean the same by "complex form" and "elaborate form" And what _do_ you mean by that? Jus that it's hard to implement? Or is it really "intractable"? Because then you talk about a "tractable simplification" ... so many big words.
---You are correct in inferring that all of these phrases are just my attempt to not be repetitively writing 'intractable' over and over again. Although, with your comments in mind, I have now combed the document for unnecessary instances of words that could be replaced by things with more meaning to the reader.
Are they all necessary for the JOSS audience to plough through, to finally learn what the software does on page 2?
---I have rearranged the paragraphs of the article to be more direct in presenting the core value of the software to the reader, before going on a tangent with what the model that underlies the software is, and its history.
Many of your DOIs are not resolving because you have a full URL instead of the DOI in the BibTeX field, so the link appears broken. Please check.
[EDIT] To clarify, the links come out as https://doi.org/https://doi.org/10.1016/S0364-0213(85)80012-4
which don't resolve.
---The http... bits of the string, up to the DOIs have now been removed from each of the references. Thank you for this correction. I didn't know how the system was handling the DOIs and thought that all of the OKs from the reference checks meant that I must have been doing the right thing.
Page 2:
All of the functions that have been comprehensively documented >> delete "that"
---'that' has been deleted.
Furthermore, the analysis of data arising from⦠>> Unclear how this sentence is connected with the previous and the need for "Furthermore"
---'Furthermore' has been removed.
complimentary >> complementary (meaning: flattering versus supportive)
---I get these two wrong all of the time. Thank you for the correction!
It's not necessary to include the GitHub repo address or the archive address in the text, because they will appear linked in the page-1 margin of the formatted paper, and will also be linked on the paper web page. Please remove.
---Github repo and archive addresses have been removed.
Overall:
I suggest a pass of ruthless editing for clarity and conciseness. For example, how does one know the difference between "comprehensively documented" and "documented," between a report that "describes" versus "clearly describes"? What is "particularly computationally intensive" versus just "computationally intensive"? When is a package "sufficiently suitable" versus "suitable"?
---You've caught me out on one of my quirks of being overly generous with adjectives and adverbs usage. The document has been combed for the over abundance of such phraseology.
What I'm trying to say, as an editor, is that the paper is too long and a slog to read, and I hope you can give it a bit of a clean.
---Point taken. I have rearranged the document so that the interesting software stuff is at the start and the more technical stuff, which I would still like to include for completeness, appears afterwards.
Also, note the Author Guideline that says to "include in the paper some sentences that would explain the software functionality and domain of use to a non-specialist reader."
---I have included the following sentence for non-specialist readers: "Simply put, the FVBM is a model that allows for the characterization of interactions between multiple binary random variables, simultaneously."
Thanks, @hiendn.
ΒΆ1
The FVBM was first in Hyvarinen (2006) >> missing verb?
@labarba haha thanks for spotting the typo; 'considered' was the missing verb. Now included. Its amazing what you end up mentally filling in when you proof read your own writing.
@whedon accept
Attempting dry run of processing paper acceptance...
```Reference check summary:
OK DOIs
MISSING DOIs
INVALID DOIs
Check final proof :point_right: https://github.com/openjournals/joss-papers/pull/494
If the paper PDF and Crossref deposit XML look good in https://github.com/openjournals/joss-papers/pull/494, 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...
π¨π¨π¨ 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...
Congratulations, @hiendn, your paper is now published!
And sincere thanks to the handling editor, @yochannah, and the reviewers, @ja-thomas, and @schalkdaniel π
: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.01193)
HTML:
<a style="border-width:0" href="https://doi.org/10.21105/joss.01193">
<img src="http://joss.theoj.org/papers/10.21105/joss.01193/status.svg" alt="DOI badge" >
</a>
reStructuredText:
.. image:: http://joss.theoj.org/papers/10.21105/joss.01193/status.svg
:target: https://doi.org/10.21105/joss.01193
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:
Most helpful comment
all done from my side :+1: