Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign up[REVIEW]: xrnet: Hierarchical Regularized Regression to Incorporate External Data #1761
Comments
This comment has been minimized.
This comment has been minimized.
Hello human, I'm @whedon, a robot that can help you with some common editorial tasks. @juanvillada, @jsgalan it looks like you're currently assigned to review this paper 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:
For example, to regenerate the paper pdf after making changes in the paper's md or bib files, type:
|
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Hi all! This is a very useful piece of software accompanied by a very well written paper. Some comments:
Thanks to the authors for open sourcing this tool! 👏🏼👏🏼 |
This comment has been minimized.
This comment has been minimized.
Hi @juanvillada, Thanks for the review! With regard to installation on macOS, I believe this is specific to recent R versions that require a more recent version of GNU Fortran to compile packages from source. See https://cloud.r-project.org/bin/macosx/tools/ for a more detailed description and a binary to install the latest GNU Fortran, which should fix the installation issue. I can also update our documentation to inform users of this detail as well. Many thanks again for your feedback! |
This comment has been minimized.
This comment has been minimized.
Hi @gmweaver, Everything works fine after installing the latest GNU Fortran.
Best wishes and thanks for this tool! |
This comment has been minimized.
This comment has been minimized.
Thanks @juanvillada for the review! @jsgalan Please post any comments about your review and complete the checklist above as soon as you are able. |
This comment has been minimized.
This comment has been minimized.
Hi guys, this review escaped from email/alerts/calendars/todo-list. I am sorry. The article explain the context very well and summarize the need for this tool and how it adds to the existing software packages.
Firstly I am using, R version 3.6.0 (2019-04-26) Platform: x86_64-apple-darwin15.6.0 (64-bit) on a MacOS Mojave 10.14.6 I tried the installation via master branch and got 17 warnings plus an installation error, see the resulting error After installing the GNUfortran, as suggested by @gmweaver, I still got 17 warnings but a clean install, see the warnings here It finally installed and I ran the demo script. I have a few suggestions for the authors:
Best, |
This comment has been minimized.
This comment has been minimized.
Thanks @jsgalan and @juanvillada for the reviews. @gmweaver, can you post your response to these comments regarding the installation process here? |
This comment has been minimized.
This comment has been minimized.
@jsgalan thanks for the review and suggestions! I have made the following changes based on your feedback:
The warnings when installing are specific to RcppEigen and although annoying, should be harmless. See https://stackoverflow.com/questions/49525561/rcppeigen-package-pragma-clang-diagnostic-pop-warnings for notes from one of the maintainers of that package (Dirk Eddelbeuttel) |
This comment has been minimized.
This comment has been minimized.
Hi @gmweaver,
Best, |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@jsgalan "x_linear.txt" is a real file stored under inst/extdata in the repository (I followed the best practices suggested in the R Packages book by Hadley Wickham for including raw data files). I have updated the readme to explain where this file is located and and how to access within R after installing the package. Let me know if this will work or if you had something else in mind for a file example. |
This comment has been minimized.
This comment has been minimized.
hey @gmweaver that sounds great, thanks for the explanation. Also, the article checks great. I am happy with the review. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Great. Thanks @jsgalan and @juanvillada for the reviews and @gmweaver for addressing all comments. @gmweaver can you fix this minor typo in the paper? Everything else looks good: Then, please make a tagged release and archive (e.g. Zenodo) and report the version number and archive DOI here. Please ensure the title and author list match that in the paper. |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@mgymrek thanks for catching that typo! Fixed and generated the PDF again. I have also released a tagged version and archived to Zenodo. The version is v0.1.1 and the Zenodo DOI is 10.5281/zenodo.3538245 |
This comment has been minimized.
This comment has been minimized.
@mgymrek small update to ensure DESCRIPTION file version matches tagged release version. New version is v0.1.2 and Zenodo DOI is 10.5281/zenodo.3538278 |
This comment has been minimized.
This comment has been minimized.
@whedon set v0.1.2 as version |
This comment has been minimized.
This comment has been minimized.
OK. v0.1.2 is the version. |
This comment has been minimized.
This comment has been minimized.
@whedon set 10.5281/zenodo.3538278 as archive |
This comment has been minimized.
This comment has been minimized.
OK. 10.5281/zenodo.3538278 is the archive. |
This comment has been minimized.
This comment has been minimized.
Thanks @gmweaver for the changes @openjournals/joss-eics we are ready to accept this article! |
This comment has been minimized.
This comment has been minimized.
Hi - sorry, this doesn't seem to have been acted on by the AEiC on duty at the time, so I will do it now. |
This comment has been minimized.
This comment has been minimized.
@whedon generate pdf |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
@danielskatz reviewed the changes in your pull request and everything looks good! Merged pull request and created a new release with changes. New version is v0.1.3 and Zenodo DOI is 10.5281/zenodo.3564788 |
This comment has been minimized.
This comment has been minimized.
@whedon set v0.1.3 as version |
This comment has been minimized.
This comment has been minimized.
OK. v0.1.3 is the version. |
This comment has been minimized.
This comment has been minimized.
@whedon set 10.5281/zenodo.3564788 as archive |
This comment has been minimized.
This comment has been minimized.
OK. 10.5281/zenodo.3564788 is the archive. |
This comment has been minimized.
This comment has been minimized.
@whedon accept |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
Check final proof If the paper PDF and Crossref deposit XML look good in openjournals/joss-papers#1160, then you can now move forward with accepting the submission by compiling again with the flag
|
This comment has been minimized.
This comment has been minimized.
@whedon accept deposit=true |
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
|
This comment has been minimized.
This comment has been minimized.
Here's what you must now do:
Any issues? notify your editorial technical team... |
This comment has been minimized.
This comment has been minimized.
Thanks to @juanvillada, @jsgalan for reviewing, and @mgymrek for editing! |
This comment has been minimized.
This comment has been minimized.
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! 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:
|
This comment has been minimized.
This comment has been minimized.
and sorry for the delay in processing this... |
whedon commentedSep 24, 2019
•
edited
Submitting author: @gmweaver (Garrett Weaver)
Repository: https://github.com/USCbiostats/xrnet
Version: v0.1.3
Editor: @mgymrek
Reviewer: @juanvillada, @jsgalan
Archive: 10.5281/zenodo.3564788
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
@juanvillada & @jsgalan, 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.readthedocs.io/en/latest/reviewer_guidelines.html. Any questions/concerns please let @mgymrek know.
Review checklist for @juanvillada
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper
Review checklist for @jsgalan
Conflict of interest
Code of Conduct
General checks
Functionality
Documentation
Software paper