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

[PRE REVIEW]: qGaussian: An R package that deal with the Tsallis statistics #229

Closed
whedon opened this issue Apr 6, 2017 · 25 comments
Closed

Comments

@whedon
Copy link

whedon commented Apr 6, 2017

Submitting author: @bunhoel (Emerson)
Repository: https://github.com/bunhoel/anvil/tree/master/qGaussian
Version: v0.1.4
Editor: @karthik
Reviewer: Pending

Author instructions

Thanks for submitting your paper to JOSS @bunhoel. The JOSS editor (shown at the top of this issue) will work with you on this issue to find a reviewer for your submission before creating the main review issue.

@bunhoel if you have any suggestions for potential reviewers then please mention them here in this thread. In addition, this list of people have already agreed to review for JOSS and may be suitable for this submission.

Editor instructions

The JOSS submission bot @whedon is here to help you find and assign reviewers and start the main review. To find out what @whedon can do for you type:

@whedon commands
@whedon
Copy link
Author

whedon commented Apr 6, 2017

Hello human, I'm @whedon. I'm here to help you with some common editorial tasks for JOSS. @jakevdp it looks like you're currently assigned as the editor for this paper 🎉

For a list of things I can do to help you, just type:

@whedon commands

@arfon
Copy link
Member

arfon commented Apr 6, 2017

👋 @jakevdp - the submitting author suggested you as the editor. Let me know if you would like to find someone else for this.

@bunhoel
Copy link

bunhoel commented Apr 7, 2017 via email

@whedon
Copy link
Author

whedon commented Apr 7, 2017

I'm sorry @bunhoel, I'm afraid I can't do that. That's something only JOSS editors are allowed to do.

@bunhoel
Copy link

bunhoel commented Apr 7, 2017 via email

@arfon
Copy link
Member

arfon commented Apr 7, 2017

👋 @cdcrabtree @masalmon @wrathematics would one of you be willing to review this submission for JOSS?

@jakevdp
Copy link

jakevdp commented Apr 17, 2017

Sorry, I somehow lost track of this among all my other github emails.

This is neither my area of expertise, nor the code community where I have connections, and so I don't think I'd be a good editor for this submission.

@arfon
Copy link
Member

arfon commented Jun 4, 2017

👋 @karthik - would you be willing to edit this submission for JOSS?

@karthik
Copy link
Contributor

karthik commented Jun 15, 2017

@whedon assign @karthik as editor

@whedon
Copy link
Author

whedon commented Jun 15, 2017

OK, the editor is @karthik

@whedon whedon assigned karthik and unassigned jakevdp Jun 15, 2017
@karthik
Copy link
Contributor

karthik commented Jun 15, 2017

Hi @bunhoel
I've done an editorial review of your submission and am sorry to inform you that your R package does not meet the JOSS standards. Your software does not meet criteria for at least the following reasons:

  • Does not contain a README (Place where things happen. is not acceptable) with a description and install instructions
  • Does not pass any package checks, and is currently not installable
  • Functions have no documentation
  • no tests

I stopped there. I would suggest that you read over the package development from Hadley Wickham (available here for free: http://r-pkgs.had.co.nz/) and fix everything up. If you are happy to go this route and ping us again when your package is ready, please let us know and I will add a pending-major-enhancements tag.

@bunhoel
Copy link

bunhoel commented Jul 14, 2017

Dear Karthik

I greatly appreciate the suggestions and I believe that all pending questions was solved.
It was included a README.md file including information about package checks and documentations.
However, the building process and package checks as made at a new
Repository: https://github.com/bunhoel/qGaussian
cloned at old
Repository: https://github.com/bunhoel/anvil/tree/master/qGaussian

Finally, I suspect that it is more productive to consider the new
repository as a work repository for further updates.

@karthik
Copy link
Contributor

karthik commented Jul 26, 2017

Hi @bunhoel,
In looking over the repo again, I have more questions for you.

  • What is the rationale for not having the package in the root of the repo? Why a subfolder? Your main README is still incomplete and says "Place where things happen". Please delete that file and move the entire qGaussian folder up one level.
  • In your code I don't see any roxygen tags or documentation at all, however in your DESCRIPTION I see RoxygenNote: 5.0.1 and in your NAMESPACE I see https://github.com/bunhoel/qGaussian/blob/master/NAMESPACE#L1. Where did these come from? Did you copy those in by hand?
  • I'm unable to install the package from your GitHub. It does not pass checks and it appears that you've generated the documentation by hand, which contain errors.

@bunhoel
Copy link

bunhoel commented Sep 1, 2017

Dear Karthik
I understand your doubts and hope to have answer the questions

  • The idea was to use subdirectories to save other R projects, but apparently this procedure was a source of problems.
    Since we can found qGaussian package at
    https://cran.r-project.org/web/packages/qGaussian/index.html
    I downloaded the file qGaussian_0.1.4.tar.gz , and after extracting the content, I upload the contents of the directories R and man to qGaussain folder as requested.

  • The "RoxygenNote: 5.0.1 and in NAMESPACE" came from the source qGaussian_0.1.4.tar.gz
    Unfortunately, I did copy the files by hand, but it was corrected.

  • I believe that after the suggested modifications that were made the package will pass the checks.

@karthik karthik added the paused label Dec 4, 2017
@karthik
Copy link
Contributor

karthik commented Dec 4, 2017

@bunhoel Can you please inform us of your plans with this package?

I downloaded the file qGaussian_0.1.4.tar.gz , and after extracting the content, I upload the contents of the directories R and man to qGaussain folder as requested.

This is not the right way to do it. You should not extract content from the built package back to source. As mentioned before, please read over the relevant chapters of Hadley Wickham's book on R packaging.

@bunhoel
Copy link

bunhoel commented Jan 9, 2018

Dear Karthik

We following the request on how to implement the R packages using the
Hadley Wickham's book (http://r-pkgs.had.co.nz/)

  • git was used to upload the package to github through a SSH connection.
  • the qGaussian repository was created and git files uploaded through the
    the "git remote add origin" and "git push -u origin master" commands as suggested by the book´s author.
  • and the DESCRIPTION file was modified by adding the github addresses and issues.

The package can now be installed by the command:
devtools :: install_github ("bunhoel / qGaussian")

Finally, it was also compiled by .travis.yml, and the qGaussian repository has now been put into the github standard.

We hopefully the package will now be in aggree the github standard.

@arfon
Copy link
Member

arfon commented Mar 28, 2018

👋@karthik - could you take a look at the response from @bunhoel above?

@karthik
Copy link
Contributor

karthik commented Apr 6, 2018

Thanks for the nudge @arfon. Taking a look now.

@karthik
Copy link
Contributor

karthik commented Apr 6, 2018

Thanks @bunhoel
The package looks ready for review now. A few comments for you to think about while I assign a reviewer.

Potential problems and suggestions for your package

  ✖ write unit tests for all functions, and all package code
    in general. 0% of code lines are covered by test cases.

    R/cqgauss_0.1.5.R:2:NA
    R/cqgauss_0.1.5.R:3:NA
    R/cqgauss_0.1.5.R:4:NA
    R/cqgauss_0.1.5.R:5:NA
    R/cqgauss_0.1.5.R:6:NA
    ... and 156 more lines

  ✖ omit "Date" in DESCRIPTION. It is not required and it
    gets invalid quite often. A build date will be added to the package
    when you perform `R CMD build` on it.
  ✖ use '<-' for assignment instead of '='. '<-' is the
    standard, and R users and developers are used it and it is easier
    to read your code for them if you use '<-'.

    R/cqgauss_0.1.5.R:1:9
    R/cqgauss_0.1.5.R:2:3
    R/cqgauss_0.1.5.R:3:4
    R/cqgauss_0.1.5.R:6:2
    R/cqgauss_0.1.5.R:7:3
    ... and 52 more lines

  ✖ avoid long code lines, it is bad for readability. Also,
    many people prefer editor windows that are about 80 characters
    wide. Try make your lines shorter than 80 characters

    R/rqgauss_0.1.5.R:5:1

  ✖ avoid 1:length(...), 1:nrow(...), 1:ncol(...),
    1:NROW(...) and 1:NCOL(...) expressions. They are error prone and
    result 1:0 if the expression on the right hand side is zero. Use
    seq_len() or seq_along() instead.

    R/cqgauss_0.1.5.R:7:11
    R/cqgauss_0.1.5.R:16:10
    R/cqgauss_0.1.5.R:25:10
    R/pqgauss_0.1.5.R:34:11
    R/pqgauss_0.1.5.R:35:10

  ✖ not import packages as a whole, as this can cause name
    clashes between the imported packages. Instead, import only the
    specific functions you need.
  ✖ not use exportPattern in NAMESPACE. It can lead to
    exporting functions unintendedly. Instead, export functions that
    constitute the external API of your package.
  ✖ fix this R CMD check WARNING: LaTeX errors when creating
    PDF version. This typically indicates Rd problems.
  ✖ fix this R CMD check ERROR: Re-running with no
    redirection of stdout/stderr. Hmm ... looks like a package You may
    want to clean up by 'rm -rf /tmp/RtmpoZPt3M/Rd2pdfae72b1ceb18'
  ✖ fix this R CMD check NOTE: Found the following hidden
    files and directories: .travis.yml 
You need to add this to .Rbuildignore
    Extensions’ manual.
────────────────────────────────────────────────────────────────────────────────
Warning messages:
1: In MYPREPS[[prep]](state, quiet = quiet) :
  Prep step for cyclomatic complexity failed.
2: In readLines(filename) :
  incomplete final line found on '/qGaussian/R/pqgauss_0.1.5.R'
3: In readLines(file) :
  incomplete final line found on '/qGaussian/R/pqgauss_0.1.5.R'

@karthik
Copy link
Contributor

karthik commented Apr 6, 2018

I spoke too soon. One remaining issue before I assign a reviewer is that you are missing unit tests. Once you add those and some code coverage, I will proceed with your submission.

@arfon
Copy link
Member

arfon commented Jun 15, 2018

👋 @bunhoel - please respond to @karthik's comments.

@bunhoel
Copy link

bunhoel commented Jul 4, 2018

Dear Karthik

we believe to fixed all potential problems as like unit test, as same time, we implement the suggestions for the package.

@karthik
Copy link
Contributor

karthik commented Jul 5, 2018

@bunhoel The package still far from us being able to send out to review:

  • The package does not contain any tests
  • The documentation is manually written Rd files that cannot be easily updated (with roxygen2 as all modern R packages follow)
  • The readme does not tell the reader how to install the development version
  • There is a package contained within the package.
  • Your paper does not adequately describe what the software does.

Package is also failing a few basic checks:

Given the the lack of progress, I'm going to pause this submission. Please ping once your package is in a state where it can be sent out for review.

@bunhoel
Copy link

bunhoel commented Jul 18, 2018

Dear Karthik,

the first thing that we did was to delete from github the directory /bun hole/anvil/tree/master/q Gaussian
because, we believe that it could be creating problems to reviews.
We believe that it solve the question:
There is a package contained within the package.

All of ours works could be found at link https://github.com/bunhoel/qGaussian

About more listed problems; test was includes, Rd files was edited by the roxygen2 and was included at readme
devtools::install_github("bunhoel/qGaussian").

Finally, paper was improved with a more details about R package and related theory.

@arfon
Copy link
Member

arfon commented Nov 2, 2018

I'm going through old reviews and this one still appears to be well below the standard we would expect to pass a JOSS review. At this point I'm going to close this review. @bunhoel - please feel free to re-submit in the future if you manage to extend/improve this submission to meet the expectations set in our review criteria

@arfon arfon closed this as completed Nov 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants