search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

Submission package: PERFect #1243

Closed cxquy91 closed 5 years ago

cxquy91 commented 5 years ago

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

Confirm the following by editing each check box to '[x]'

I am familiar with the essential aspects of Bioconductor software management, including:

For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.

bioc-issue-bot commented 5 years ago

Hi @cxquy91

Thanks for submitting your package. We are taking a quick look at it and you will hear back from us soon.

The DESCRIPTION file for this package is:

Package: PERFect
Type: Package
Title: Permutation filtration for microbiome data
Version: 0.2.1
Date: 2019-09-02
Author: 
    Ekaterina Smirnova <ekaterina.smirnova@vcuhealth.org>,
      Quy Cao <quy.cao@umontana.edu>
Maintainer: Quy Cao <quy.cao@umontana.edu>
Description: 
    PERFect is a  novel permutation filtering approach designed to address
    two unsolved problems in microbiome data processing: (i) define and  quantify 
    loss due to filtering by implementing thresholds, and (ii) introduce and evaluate
    a permutation  test for  filtering loss to provide a measure of excessive filtering.
Depends: R (>= 3.6.0), sn (>= 1.5.2)
Imports: ggplot2 (>= 3.0.0), phyloseq (>= 1.28.0), zoo (>= 1.8.3),
        psych (>= 1.8.4), stats (>= 3.6.0), Matrix (>= 1.2.14),
        fitdistrplus (>= 1.0.12)
License: Artistic-2.0
Encoding: UTF-8
LazyData: false
BugReports: https://github.com/katiasmirn/PERFect/issues
URL: https://github.com/katiasmirn/PERFect
Suggests: knitr, rmarkdown, kableExtra, ggpubr
biocViews: Software, Microbiome, Sequencing, Classification,
        Metagenomics
VignetteBuilder: knitr
RoxygenNote: 6.1.1
NeedsCompilation: no
Packaged: 2019-08-28 20:38:07 UTC; CaoQ1

Add SSH keys to your GitHub account. SSH keys will are used to control access to accepted Bioconductor packages. See these instructions to add SSH keys to your GitHub account.

bioc-issue-bot commented 5 years ago

A reviewer has been assigned to your package. Learn what to expect during the review process.

IMPORTANT: Please read the instructions for setting up a push hook on your repository, or further changes to your repository will NOT trigger a new build.

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, ERROR". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

cxquy91 commented 5 years ago

Hi,

I have fixed the issue from the build report and update the github repository.

Thank you,

Quy

lshep commented 5 years ago

Please set up the webhook as described here . Once that is set up perform a version bump in the DESCRIPTION and it will trigger a new build report.

cxquy91 commented 5 years ago

Hi Ishep, I couldn't find the "Settings" menu on the landing page of https://github.com/katiasmirn/PERFect (eventhough we are both contributors to the package) so I forked it, updated the repository link to https://github.com/cxquy91/PERFect, added the webhook and bumped the version.

lshep commented 5 years ago

Are you sure you want to continue with the location https://github.com/cxquy91/PERFect instead of https://github.com/katiasmirn/PERFect ? I will need it updated in our SPB database if that is the case which is why the version bump didn't work this time - and this will be the canonical location of the package and where we will pull key information from once accepted.

cxquy91 commented 5 years ago

Yes, it would be better to use the location https://github.com/cxquy91/PERFect because I will have more control over this repo.

lshep commented 5 years ago

It has been updated in our database. Please set up the webhook, and once that is set please try to do a version bump to see if the build gets triggered.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

0f608ba Version 0.2.4 0c30f15 Merge pull request #2 from katiasmirn/master Vers...

cxquy91 commented 5 years ago

I just update to another version and it seems that the bot is building.

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

9a3f9c3 test 5321aaa Merge branch 'master' of https://github.com/cxquy9...

cxquy91 commented 5 years ago

The most recent error came from the .Rproj file, so I think the bot is complaining about my Git/SVN url (it was https://github.com/katiasmirn/PERFect, but I just changed into https://github.com/cxquy91/PERFect). I just bump to another version. Let's see what the bot says.

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

cxquy91 commented 5 years ago

The bot still doesn't like how I update my PERFect.Rproj. Please let me know how should I do this correctly. This is what I have done:

Follow instruction from https://git.wiki.kernel.org/index.php/GitSvnSwitch to change Git/Svn for my package, using the General Case guidelines. (doesn't seem to work). This is what I got that makes me think it doesn't work:

$ git svn fetch Migrating from a git-svn v1 layout... Data from a previous version of git-svn exists, but .git/svn (required for this version (2.23.0.windows.1) of git-svn) does not exist. Done migrating from a git-svn v1 layout [svn-remote "svn"] unknown

And this is my updated .git/config:

[core] repositoryformatversion = 0 filemode = false bare = false logallrefupdates = true symlinks = false ignorecase = true [remote "origin"] url = https://github.com/cxquy91/PERFect fetch = +refs/heads/:refs/remotes/origin/ [branch "master"] remote = origin merge = refs/heads/master [gui] wmstate = normal geometry = 893x435+234+234 175 196

lshep commented 5 years ago

That ERROR i indicating that the .Rproj file should not be git tracker. In order to remove the file you need to do the following git rm PERFect.Rproj You can keep this file for yourself but do not commit it to git (this can be done with a .gitignore file). After you do the git rm, do a version bump, git commit the changes and the ERROR should be corrected.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

3e4d5e9 Version 0.2.6

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

b35cf43 Version 0.2.7

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ABNORMAL". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "ERROR, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

7202fdf Version 0.99.8 d68b943 Version 0.99.9

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

On one or more platforms, the build results were: "skipped, TIMEOUT, WARNINGS". This may mean there is a problem with the package that you need to fix. Or it may mean that there is a problem with the build system itself.

Please see the build report for more details.

cxquy91 commented 5 years ago

Should I be worry about this TIMEOUT? It looks like Linux system struggles to build it while other two are fine. Also, should I remove inst/doc folder?

Kayla-Morrell commented 5 years ago

This is most likely an issue on our end so I wouldn't worry about the TIMEOUT for now. Yes, please remove the inst/doc folder.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

cb700b7 Version 0.99.10 no inst/doc dcc6f66 Merge branch 'master' of https://github.com/cxquy9...

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

cxquy91 commented 5 years ago

finally :D

Kayla-Morrell commented 5 years ago

Hello @cxquy91,

Thank you for submitting to Bioconductor. Please see the initial review of the package below. The required changes must be made while the suggested changes do not have to be but we strongly encourage them. Comment back here with updates that have been made and when the package is ready for a re-review.

General package development

DESCRIPTION

NEWS

Data

Vignettes

if(!requireNamespace("BiocManager", quietly = TRUE))
    install.packages("BiocManager")
BiocManager::install("PERFect")

man pages

PERFect_perm

PERFect_perm_reorder

knight

Unit tests

R code

Best, Kayla

Kayla-Morrell commented 5 years ago

@cxquy91 Just wanted to check in since I saw this issue was closed. Is there a reason you closed the issue?

cxquy91 commented 5 years ago

Oh no, I didn't notice that I close it. I was reviewing the your suggestions and I might accidentally hit the close button. I am almost finished with the revision and will update package. Thank you for noticing that.

Kayla-Morrell commented 5 years ago

Hello @cxquy91,

Just checking in to see if there are any updates on the initial review for this package? Please keep in mind that if you would like this package to be included in the upcoming 3.10 release the review process must be finished and the package accepted by October 23rd.

Best, Kayla

cxquy91 commented 5 years ago

Dear Kayla,

Thank you for the reminder. I was a little busy recently but I will try to push this package out in the next few days.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

3fb52e8 Test on new laptop

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

76c120a First update version 0.99.12 dabe057 Merge branch 'master' of https://github.com/cxquy9...

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

Kayla-Morrell commented 5 years ago

@cxquy91 - Is the package ready for re-review now or are you planning on making more changes?

cxquy91 commented 5 years ago

Hi Kayla,

I will have another push by tonight it will be for review. Thank you,

cxquy91 commented 5 years ago

General package development • REQUIRED: The build/ directory does not seem necessary and should be removed. It is removed. DESCRIPTION • REQUIRED: The 'Packaged:' tag is not needed and should be removed. It is removed. • REQUIRED: There are functions used in your code from 'parallel' and it is not listed in the 'Imports:' field. Please add it. “parallel” is added. NEWS • REQUIRED: Thank you for including a NEWS file, this very helpful when tracking changes in the code from one version to the next. However, it does not look like your file is formatted correctly. Please see the following link on how to properly format your NEWS file: https://bioconductor.org/developers/package-guidelines/#news. I have reformatted to the way it should be. Data • CLARIFICATION: I see the use of the data files 'mock' and 'mock2' within the vignette and the man pages. Where are the other data files used? Other data are used for other demonstrations, but those vignettes must be cleaned up and updated before added in. Therefore, I temporary removed them for now and added them in once their vignettes are completed. Vignettes • REQUIRED: Lines 27-36, most of this code needs to be shown to the user and should be incorporated into the workflow code. The libraries should be loaded where needed and the user needs to know what to set the seed to in order to produce the same results as you did. I have shown them in the vignette. • REQUIRED: The 'Installation' section should demonstrate to the user how to download and install the package from Bioconductor. The code should look something like the following and include 'eval=FALSE': if(!requireNamespace("BiocManager", quietly = TRUE)) install.packages("BiocManager") BiocManager::install("PERFect") This is also added. • SUGGESTION: In the 'Introduction' section you might explain what a taxa is since the user maybe unfamiliar with this type of data. I added a brief discussion about taxa and filtering. • REQUIRED: Lines 290-300, something doesn't seem right in here. When I generate the vignette on my computer the math doesn't seem to line up properly and there are two trailing 'end's that don't seem to belong here. Just make sure all of the code is correct. This supposes to mimic the for loop, but I can see that it is confused so I removed them and reformat the loop. • REQUIRED: Line 416-428, the same goes for this chunk of code as well. When I generate the vignette the math doesn't line up properly and there is a trailing 'end' that doesn't seem to belong at the end of the paragraph. Double check that the code is correct. Same as above. • REQUIRED: Line 467-474, same as above with one trailing 'end'. Same as above • REQUIRED: The last section of the vignette should be "Session Information" and should include sessionInfo(). SessionInfo() is added.

• SUGGESTION: You give a very thorough illustation and explanation of the method and a very little section about the actual use of the package. Keep in mind the purpose behind a vignette, to demonstrate how to accomplish non-trivial tasks embodying the core functionality of the package. My suggestion would be to shorten the explanation of the method a bit and include a little bit more of the functionality of the package. I have added extra examples for user to better manipulate both functions. man pages PERFect_perm • SUGGESTION: The last part of your example could be contained in within \dontrun{} so that the user won't have to uncomment out the code. The code will show regularly withing a don't run section and it will not be run when testing example code. Thank you for this suggestion. It is very helpful and I have implemented it. PERFect_perm_reorder • SUGGESTION: The same suggestion can be applied to the end of the examples in this man page as well. Same as above. knight • REQUIRED: This data file is incomplete. This is removed. Unit tests • SUGGESTION: Consider adding unit tests. We strongly encourage them. I have different files that test my functions so this is not necessary. R code • REQUIRED: Avoid sapply(); use vapply() instead. Found in files:

This is where I spent most of the time tinkering. I have changed all the loops wherever applicable, but there are some instances I have to use loops. It is either because the index of the loop is coded as part of the function, such as the functions DiffFiltLoss_j, Perm_j_s in the utility_functions.R, or I am running a binary search, so I don’t want to execute my loops for all taxa in order to be more computationally efficient. Since apply functions would essentially execute for all taxa, it would not be applicable.

• SUGGESTION: Consider shorter lines, 517 lines are > 80 characters long. I hope it is not a big deal if I kept them unchanged. • SUGGESTION: Consider multiples of 4 spaces for line indents, 889 lines are not. I hope it is not a big deal if I kept them unchanged.

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

2c6cabb Version 0.99.13. Ready for re-review.

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

Kayla-Morrell commented 5 years ago

Hello @cxquy91 - Thank you for making the necessary changes and clarifying where needed. I have just one additional note before accepting the package. Please see this requirement below.

Vignette

Best, Kayla

bioc-issue-bot commented 5 years ago

Received a valid push; starting a build. Commits are:

820f8b5 Version 0.99.14. Second revision.

cxquy91 commented 5 years ago

Hi Kayla,

Thank you for spotting that. I just quickly fixed the issue and push the updated package.

Quy

bioc-issue-bot commented 5 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be posed to this issue in the next several days.

Thank you for contributing to Bioconductor!

bioc-issue-bot commented 5 years ago

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on Linux, Mac, and Windows.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

mtmorgan commented 5 years ago

The master branch of your GitHub repository has been added to Bioconductor's git repository.

To use the git.bioconductor.org repository, we need an 'ssh' key to associate with your github user name. If your GitHub account already has ssh public keys (https://github.com/cxquy91.keys is not empty), then no further steps are required. Otherwise, do the following:

  1. Add an SSH key to your github account
  2. Submit your SSH key to Bioconductor

See further instructions at

https://bioconductor.org/developers/how-to/git/

for working with this repository. See especially

https://bioconductor.org/developers/how-to/git/new-package-workflow/ https://bioconductor.org/developers/how-to/git/sync-existing-repositories/

to keep your GitHub and Bioconductor repositories in sync.

Your package will be included in the next nigthly 'devel' build (check-out from git at about 6 pm Eastern; build completion around 2pm Eastern the next day) at

https://bioconductor.org/checkResults/

(Builds sometimes fail, so ensure that the date stamps on the main landing page are consistent with the addition of your package). Once the package builds successfully, you package will be available for download in the 'Devel' version of Bioconductor using BiocManager::install("PERFect"). The package 'landing page' will be created at

https://bioconductor.org/packages/PERFect

If you have any questions, please contact the bioc-devel mailing list (https://stat.ethz.ch/mailman/listinfo/bioc-devel); this issue will not be monitored further.