Closed anfederico closed 5 years ago
Received a valid push; starting a build. Commits are:
5e23607 Fix error
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, 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.
Received a valid push; starting a build. Commits are:
3516d0a Fix docs and vignette #35
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.
Received a valid push; starting a build. Commits are:
abc986e run_animalcules add example
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.
Received a valid push; starting a build. Commits are:
0abc004 Change pathoscope functions to internal
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, 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.
Received a valid push; starting a build. Commits are:
fb2017c Fix hanging on check examples
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.
Hi @Kayla-Morrell
Thank you for the feedback. We have addressed the following issues. Please see our responses and clarifications in bold/italics. One note, we are receiving one warning on the build report but we can't figure out what's causing it. Both devtools::check() and BioCheck() are running on our machines with no errors.
R CMD BiocCheck
[x] Incomplete final line in inst/shiny/utils/ui_util.R
[x] Consider shorter lines, 157 lines are 80 characters
[x] Consider 4 spaces instead of tabs, 1 line contains tabs
[x] Consider multiples of 4 spaces for line indents, 666 lines are not (FormatR for help)
[x] REQUIRED: Clean up repository to only include files that are required to build the package ('doc', '.travis.yml', etc. are not needed).
[x] REQUIRED: 'LazyData' should be set to 'FALSE'. Having it set to 'TRUE' only slows down the loading of packages with data.
[x] CLARIFICATION: Make sure the following packages should be imported since they aren't listed in NAMESPACE
[x] REQUIRED: Add 'methods' and 'stats' to 'Imports'.
[x] REQUIRED: 'BiocCheck' and 'devtools' do not have to be suggested unless they are used in examples/vignettes.
[x] REQUIRED: 'Software' related biocViews need to be included in this package. Right now you are using 'ExperimentData' tags.
[x] REQUIRED: Exported functions should use camel case or underscoring, not '.' since this indicates S3 dispatch.
[x] SUGGESTION: Be sure that you are only using import()
if many function are used from the package. If only a few functions are used than import them using importFrom()
.
[x] REQUIRED: If you intend to use a NEWS file then please fill it in. See https://bioconductor.org/developers/package-guidelines/#news for how to format properly.
[x] REQUIRED: Data in '/data' needs to be documented.
[x] CLARIFICATION: What is the purpose of the file in the 'data-raw' directory. The script in this directory is now the script used to create MAE.rda in the data directory
[X] REQUIRED: Documentation for the 'inst/extdata' should be included in the 'inst/script' directory. This should be a script that clearly present how the data was generated. It should include source urls and any important information regarding filtering or processing. It can be executable code, sudo code, or text description. The user should be able to download and be able to roughly reproduce the file or object that is present.
[X] REQUIRED: We prefer to have an 'Introduction' section that serves as an abstract. Remove the 'abstract:' tag from the metadata of the vignette and create an 'Introduction' section with it. Also be sure to include
around the package name so it will be formated as code instead of regular text.
[X] REQUIRED: Include an 'Installation' section that shows the users how to download and load the package from Bioconductor.
[X] REQUIRED: The knit options does not have to be a section in your vignette that displays to the user. Add include = FALSE
inside the {r}
so that the code is run but will not be shown in the vignette output.
[X] SUGGESTION: It can be up to you what you include in the knit options but I would suggest not including message = FALSE
for all code. Messages can be helpful to users as they work through the vignette. Instead I would add this option to the specific chunks of R code that you won't want the message to show. I would suggest adding in the knit options things like collapse = TRUE
and comment = "#"
. We added comments but we actually don't want to show messages on any code blocks.
[X] REQUIRED: You do not need to load in all of the libraries for your package in your vignette. They will be loaded when they install your package. Please remove this chunk (lines 47-69). The only libraries that should be loaded are the ones that aren't imported and used strictly in your vignette. Then I would load them as needed in the code.
[X] REQUIRED: Line 88-90, this code should not be commented out. Instead, add the tag eval = FALSE
to {r} so that the code is shown but not evaluated when building the vignette.
[X] CLARIFICATION: After running the run_animalcules()
function I noticed on the opening page of the shiny app there is no information for the 'Annotation table'. Both 'Counts Table' and 'Taxonomy Table' have descriptive information about them but 'Annotation' does not. I'm not sure if this was intended but it might be something to consider adding. Added relevant information.
[X] SUGGESTION: Line 132, I would do head(result$sam_table)
so that instead of all 50 subjects being shown just the first 6 subjects.
[X] SUGGESTION: I wouldn't comment out line 259.
[X] SUGGESTION: I wouldn't comment out line 275.
[X] SUGGESTION: Line 276, I would also do head(result$table)
here so all subjects aren't printed out.
[X] CLARIFICATION: For line 324, when I run the code myself is it expected to get a different list of biomarkers than the ones you show? This will then change the plots following. If so, this might be something you point out to the user so they don't think they messed something up.
[X] REQUIRED: Section 11 - Toy Dataset, this section is not needed and is what should be included in the 'inst/script' directory. This demonstrates how you went from 'animalcules.rds' to 'MAE.rds.
MAE
[x] REQUIRED: This needs to be documentation needs to be more complete. Take a look at http://r-pkgs.had.co.nz/data.html#data-data for help with this. See R/data.R
animalcules-package
[X] SUGGESTION: You might consider adding some links to the main function in this page.
diversities
[x] REQUIRED: For arguments, the description of 'zeroes' seems incomplete. Not sure if there needs to be a 'TRUE' or 'FALSE' somewhere in the sentence.
_diversitieshelp
[x] CLARIFICATION: How is this different from 'diversities'? _Diversities is the general function that will check user conditions from the Shiny application and then decide if it should call diversitieshelp, which will calculate the diversity.
_filter_summarytop
[x] REQUIRED: Could you change the variable being plotted in the example to show a pie chart and not a boxplot?
_find_taxonmat
[x] REQUIRED: Add runnable examples since it is an exported function.
_read_pathoscopedata
[X] REQUIRED: Add runnable examples since it is an exported function.
animalcules
[x] CLARIFICATION: Is the title/description correct for this file?
[x] REQUIRED: Add runnable example since it is an exported function (it will most likely be the same as 'Usage' but it's good practice to have since some users just go right to examples).
_upsamplecounts
[x] REQUIRED: Add runnable examples since it is an exported function.
[x] REQUIRED: Use vapply()
instead of sapply()
. The benefit of using vapply over sapply is not apparent to me, and we would have to change a lot of code, for seemingly no improvement to the code base
[x] REQUIRED: Use various apply functions instead of for
loops. Sometimes for loops are a better choice than apply functions. For example when adding many traces to plotly objects.
[x] REQUIRED: Use seq_len()
or seq_along()
instead of 1:...
.
[x] SUGGESTION: If using import
or importFrom
then you don't need to use package::function
in the R code, you can just use the function normally. This will help simplify your code a bit.
[x] REQUIRED: The functions in 'utils.R' don't need to be exported to the user. Since they are documented already you can add the @keywords interal
which will remove them from the package index. Some of these functions are useful for microbiome transcriptomics data analysis and therefore should be accessible to users.
Hello @anfederico,
Thank you for all the work done on this package. I have looked over the changes made and have just a few additional notes before accepting your package. See the secondary review below and comment back here when you are ready for me to look it over again.
R CMD check
R CMD BiocCheck
[ ] REQUIRED: I asked that you clarify that certain packages should be listed as an 'Import' even though they were not listed in NAMESPACE. I was able to locate instances of 'S4Vectors', 'shiny' and 'vegan' within the Shiny R code but I still cannot locate where 'tibble', 'ape', or 'scales' are used. Please indicate where they are used or remove them from 'Imports'.
[ ] REQUIRED: 'methods' and 'stats' where not added to 'Imports:' though you marked that they were. Please add them.
[ ] SUGGESTION: Consider adding 'Coverage' to 'biocViews:' if applicable.
importFrom("methods", "as")
and importFrom("stats", "as.formula", "cor.test", "density", "kruskal.test", "model.matrix", "quantile", "t.test", "wilcox.test")
should be added to NAMESPACE file.[ ] REQUIRED: Thank you for including documentation for the data in 'inst/script'. There needs to be documentation though for where 'animalcules.rds' and 'TB_example_dataset.rds' come from.
[ ] REQUIRED: Where is 'toy_data.rds' actually used?
[ ] REQUIRED: Line 36, 'animalcules' should be lowercase even if it is at the beginning of a sentence.
[ ] REQUIRED: Line 31, 'devtools::load_all(".")' is not needed and should be removed.
read_pathoscope_data
[ ] REQUIRED: No runnable examples were added to this file.
[ ] CLARIFICATION: We recommend that vapply()
be used instead of sapply()
because problems arixe when the 'X' argument to sapply()
has length 0. The return type is then a list()
rather than a vector or array. There are only 3 instances of sapply()
in your code:
[ ] REQUIRED: I see you made the seq_along()
or seq_len()
changes to your R code but this also needs to be reflected in the shiny R code. Please make the necessary changes.
Best, Kayla
Received a valid push; starting a build. Commits are:
5337fb1 fix a few issues
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, 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.
Received a valid push; starting a build. Commits are:
bdfff84 fix issues
Received a valid push; starting a build. Commits are:
3ac06a9 Rm meta
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.
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.
Hi @Kayla-Morrell
Thank you again for the feedback. We have addressed the following concerns.
R CMD check
R CMD BiocCheck
We fixed as many as we could!
[x] REQUIRED: I asked that you clarify that certain packages should be listed as an 'Import' even though they were not listed in NAMESPACE. I was able to locate instances of 'S4Vectors', 'shiny' and 'vegan' within the Shiny R code but I still cannot locate where 'tibble', 'ape', or 'scales' are used. Please indicate where they are used or remove them from 'Imports'. _The tibble package is used in find_biomarker.R and has been added to its imports. The ape package is used in dimred_pcoa.R and has been added to its imports. The scales packaged is used in dimred_pca.R and dimredpcoa.R and has been added to its imports.
[x] REQUIRED: 'methods' and 'stats' where not added to 'Imports:' though you marked that they were. Please add them. Added
[x] SUGGESTION: Consider adding 'Coverage' to 'biocViews:' if applicable. Thanks for the suggestion. Added
importFrom("methods", "as")
and importFrom("stats", "as.formula", "cor.test", "density", "kruskal.test", "model.matrix", "quantile", "t.test", "wilcox.test")
should be added to NAMESPACE file.[x] REQUIRED: Thank you for including documentation for the data in 'inst/script'. There needs to be documentation though for where 'animalcules.rds' and 'TB_exampledataset.rds' come from. Added_
[x] REQUIRED: Where is 'toydata.rds' actually used? It is currently being used in the examples for R/upsample_counts._
[x] REQUIRED: Line 36, 'animalcules' should be lowercase even if it is at the beginning of a sentence. Fixed.
[x] REQUIRED: Line 31, 'devtools::loadall(".")' is not needed and should be removed. Fixed._
read_pathoscope_data
[x] REQUIRED: No runnable examples were added to this file. This function shouldn't have been exported, we have changed it to internal instead. It will eventually be integrated into and run only from the Shiny application.
[x] CLARIFICATION: We recommend that vapply()
be used instead of sapply()
because problems arixe when the 'X' argument to sapply()
has length 0. The return type is then a list()
rather than a vector or array. There are only 3 instances of sapply()
in your code: Fixed.
[x] REQUIRED: I see you made the seq_along()
or seq_len()
changes to your R code but this also needs to be reflected in the shiny R code. Please make the necessary changes. Fixed.
@anfederico - Thank you again for making the necessary changes. I will be able to accept the package once the ERROR is cleaned (and ideally the WARNINGS as well).
Best, Kayla
@Kayla-Morrell
The error message seems to be an http request issue from using the rentrez package. My best guess is it is specific to the linux machine being used by Bioconductor to check the examples. I'm not sure what I could change on our end to solve this error (besides adding skip example to the roxygen), as it should work on most machines (ours included).
Error in curl::curl_fetch_memory(url, handle = handle) : Error in the HTTP2 framing layer Calls: find_taxonomy ... request_fetch -> request_fetch.write_memory ->
The warnings are from this. I saw a few users mentioning the same warning on the Bioconductor mailing list, and responses saying that it was an internal issue of Bioconductor. And to ignore it.
Found the following significant warnings: Warning: multiple methods tables found for 'rowSums' Warning: multiple methods tables found for 'colSums' Warning: multiple methods tables found for 'rowMeans' Warning: multiple methods tables found for 'colMeans'
@anfederico - After talking over with my colleagues we are going to go ahead and accept the package. It seems to be a recent ERROR and a possible server issue that we hope will fix itself. We would rather see the code run in the example to be sure things aren't broken in the future then not have it run/test. Thank you for all of your work on this package.
Best, Kayla
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!
@Kayla-Morrell Thank you!
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/anfederico.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("animalcules")
. The package 'landing page' will be created at
https://bioconductor.org/packages/animalcules
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.
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]'
[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:
For help with submitting your package, please subscribe and post questions to the bioc-devel mailing list.