lefser - Githubissues

asyakhl commented 4 years ago

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

Repository: https://github.com/waldronlab/lefser

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

[x] I understand that by submitting my package to Bioconductor, the package source and all review commentary are visible to the general public.
[x] I have read the Bioconductor Package Submission instructions. My package is consistent with the Bioconductor Package Guidelines.
[x] I understand that a minimum requirement for package acceptance is to pass R CMD check and R CMD BiocCheck with no ERROR or WARNINGS. Passing these checks does not result in automatic acceptance. The package will then undergo a formal review and recommendations for acceptance regarding other Bioconductor standards will be addressed.
[x] My package addresses statistical or bioinformatic issues related to the analysis and comprehension of high throughput genomic data.
[x] I am committed to the long-term maintenance of my package. This includes monitoring the support site for issues that users may have, subscribing to the bioc-devel mailing list to stay aware of developments in the Bioconductor community, responding promptly to requests for updates from the Core team in response to changes in R or underlying software.

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

[x] The 'devel' branch for new packages and features.
[x] The stable 'release' branch, made available every six months, for bug fixes.
[x] Bioconductor version control using Git (optionally via GitHub).

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

bioc-issue-bot commented 4 years ago

Hi @asyakhl

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: lefser
Type: Package
Title: R implementation of the LEfSE method for microbiome biomarker discovery
Description: lefser uses Kruskal-Wallis test, Wilcoxon-Rank Sum test, and LDA
  to find biomarkers of groups and sub-groups.
Version: 0.99.0
Authors@R: c(
    person("Asya", "Khleborodova", email = "asya.bioconductor@gmail.com",
        role = c("cre", "aut")),
    person("Ludwig", "Geistlinger", email = "Ludwig.Geistlinger@sph.cuny.edu",
        role = "ctb"),
    person("Marcel", "Ramos", email = "marcel.ramos@roswellpark.org",
        role = "ctb", comment = c(ORCID = "0000-0002-3242-0582")),
    person("Levi", "Waldron", email ="levi.waldron@sph.cuny.edu", role = "ctb")
    )
License: Artistic-2.0
LazyData: true
Depends:
    SummarizedExperiment,
    R (>= 3.6.0)
Imports:
    coin,
    MASS,
    ggplot2,
    methods
Suggests: 
    knitr,
    rmarkdown,
    curatedMetagenomicData,
    BiocStyle,
    testthat,
    pkgdown,
    covr
Encoding: UTF-8
VignetteBuilder: knitr
biocViews: Software, Sequencing, DifferentialExpression, Microbiome, StatisticalMethod, Classification
RoxygenNote: 7.1.1

bioc-issue-bot commented 4 years ago

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

IMPORTANT: Please read this documentation for setting up remotes to push to git.bioconductor.org. It is required to push a version bump to git.bioconductor.org to trigger a new build.

Bioconductor utilized your github ssh-keys for git.bioconductor.org access. To manage keys and future access you may want to active your Bioconductor Git Credentials Account

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 18a195d2eabd6ac7dc202514ea04f37a3113912f

bioc-issue-bot commented 4 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: "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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: ffc8d4d718779d78964543658e660d4c34cade3f

bioc-issue-bot commented 4 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: "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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 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: "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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 29a4ca0d18d13bb1db58390e73f850b037850b4d

bioc-issue-bot commented 4 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 97287deb1724a0a633db6443796743297d42f407

bioc-issue-bot commented 4 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 4 years ago

Hi Martin, @mtmorgan We'd like to have this package in release. Thank you. -Marcel

mtmorgan commented 4 years ago

Only a few comments on this package

DESCRIPTION

OK

vignettes

lefser.Rmd:66 Please explain this comment!!!!! It is the only line in the vignette that evaluates package function.

R

data.R:5 -- it's not so useful to provide user documentation that indicates directory data-raw, since this is not available to the user. Better to put these scripts in inst/script and reference the location using system.file().
data.R:11 this looks like an incomplete description of the data; please complete.
lefser.R:63 I'm not sure whether the size of data makes efficiency important, but this code looks very inefficient. Start by 'hoisting' common subsexpressions outside loops
```
c <- 0
for (i in seq_along(unique(block))) {
    llosc <- logical.list.of.subclasses[[i]]]
    n_i <- rep(0, sum(logical.list.of.subclasses[[i]]))
```

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 829479b33d621daebe6713a079c89cdd9aa592b9

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 4146ee895ce4719c8fd42d3574885df90d583018

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 78b38e96d9fa381d1cfd0bb515b5dc69500fa6ea

LiNk-NY commented 4 years ago

Hi Martin, @mtmorgan Thank you for your comments and the quick review. Here is our line-by-line response.

vignettes

lefser.Rmd:66 Please explain this comment!!!!! It is the only line in the vignette that evaluates package function.

We've updated the main lefser function and this code chunk is now running. We also removed development comments.

R

data.R:5 -- it's not so useful to provide user documentation that indicates directory data-raw, since this is not available to the user. Better to put these scripts in inst/script and reference the location using system.file().

We've moved this script to inst/scripts.

data.R:11 this looks like an incomplete description of the data; please complete.

We weren't sure if you wanted us to document all columns in the data (~25) so we provided descriptions only for the relevant ones in the analysis.

lefser.R:63 I'm not sure whether the size of data makes efficiency important, but this code looks very inefficient. Start by 'hoisting' common subsexpressions outside loops
c <- 0
for (i in seq_along(unique(block))) {
  llosc <- logical.list.of.subclasses[[i]]]
  n_i <- rep(0, sum(logical.list.of.subclasses[[i]]))

We re-worked many of the for loops in the code based on this comment and saw a marked speed increase.

See the approximate changes in this comparison: https://github.com/waldronlab/lefser/compare/97287deb1724a0a633db6443796743297d42f407..master

PS. Unit tests are failing in the package due to an unknown error. When run locally, the unit tests are successful but when running R CMD check they fail. We are not sure if this is related to the seed generation. We used withr::with_seed to see if this would help but we get the same result as using set.seed before the function call (see above report). Any insight would be appreciated.

Thank you.

-Marcel cc: @asyakhl

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 62700c8db44f3525eef22ff15f7f889b79493ac7

LiNk-NY commented 4 years ago

Hi Martin, @mtmorgan

It seems like the tests are passing on GitHub Actions (r79342) but not locally on the r78887 or r79342 checkouts of R.

GHA: R 4.0.3 patched -- OK https://github.com/waldronlab/lefser/actions/runs/321105511 Local: R 4.0.2 patched -- FAILS Local: R 4.0.3 patched -- FAILS SPB: R 4.0.3 (2020-10-10) -- FAILS

I should also note that when the tests are run in an R session, they do pass. The value of the seed is 1 and the documentation says

It is intended as a simple way to get quite different seeds by specifying small integer arguments, and also as a way to get valid seed sets for the more complicated methods (especially '"Mersenne-Twister"' and '"Knuth-TAOCP"').

I am not sure if this makes a difference.

I will keep an eye out on the latest report. Any ideas are appreciated. Thanks!

mtmorgan commented 4 years ago

I think the issue is the collation (sort) order. If I modify the start of the test_that() clause to

test_that("lefser and lefserPlot work", {
  print(Sys.getlocale())

I see

> Sys.getlocale()
[1] "en_US.UTF-8/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8"
> devtools::test()
Loading lefser
Testing lefser
✔ |  OK F W S | Context
⠏ |   0       | lefser[1] "C/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8"

so devtools::test() has changed the collation order from "en_US.UTF-8" to "C". And indeed setting the collation order and stepping through the individual lines fo the test...

> Sys.setlocale("LC_COLLATE", "C")
[1] "C"
> Sys.getlocale()
[1] "C/en_US.UTF-8/en_US.UTF-8/C/en_US.UTF-8/en_US.UTF-8"
>   dataenv <- new.env(parent = emptyenv())
>   data("zeller14", package = "lefser", envir = dataenv)
>   zeller14 <- dataenv[["zeller14"]]
>   tol <- 0.2
>   zellersub <- zeller14[1:150, zeller14$study_condition != "adenoma"]
>   expect_error(lefser(zellersub, groupCol = NULL, blockCol = NULL))
>   results <- withr::with_seed(1,
+     lefser(zellersub, groupCol = "study_condition", blockCol = NULL)
+   )
>   expect_equal(colnames(results), c("Names", "scores"))
>   expect_true(checkEnding(results, 1, "Names", "p__Firmicutes`"))

Error: checkEnding(results, 1, "Names", "p__Firmicutes`") isn't true.

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 4e58ece3e5fca28c264a83286335901406d965e2

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

LiNk-NY commented 4 years ago

Thanks Martin @mtmorgan for the major tip!

We've made the tests locale-specific. We will continue to work on the tests and compare results to the LEfSe implementation on Galaxy. The error in the build report has to do with the changes to the support site.

cc: @lshep @Kayla-Morrell

* Checking for support site registration...
Warning in if (suppressMessages(content(response))) { :
  the condition has length > 1 and only the first element will be used
Error in if (suppressMessages(content(response))) { : 
  argument is not interpretable as logical
Calls: <Anonymous> -> BiocCheck -> checkForSupportSiteRegistration

lshep commented 4 years ago

Yes we are working on the fix now. Sorry for the inconvenience.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 1fcfff3ac1d355c0f7a081116f0851cbca32c859

bioc-issue-bot commented 4 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". 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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 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: "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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Received a valid push on git.bioconductor.org; starting a build for commit id: 4df78a859b18231f103ed96d76476f8ea290d71c

asyakhl commented 4 years ago

Hi Martin, @mtmorgan lefser is ready for a second look.

Thank you, Asya cc: @LiNk-NY

bioc-issue-bot commented 4 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: "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. This link will be active for 21 days.

Remember: if you submitted your package after July 7th, 2020, when making changes to your repository push to git@git.bioconductor.org:packages/lefser to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

bioc-issue-bot commented 4 years ago

Your package has been accepted. It will be added to the Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

bioc-issue-bot commented 4 years ago

Your package has been accepted. It will be added to the Bioconductor nightly builds.

Thank you for contributing to Bioconductor!

mtmorgan commented 4 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/asyakhl.keys is not empty), then no further steps are required. Otherwise, do the following:

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("lefser"). The package 'landing page' will be created at

https://bioconductor.org/packages/lefser

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.

Bioconductor / Contributions

lefser #1695