Caleb-Huo commented 5 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/Caleb-Huo/AWFisher

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 5 years ago

Hi @Caleb-Huo

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: AWFisher
Type: Package
Title: An R package for fast computing for adaptively weighted fisher's method
Version: 0.99.0
Date: 2016-12-14
Author: Zhiguang Huo
Maintainer: Zhiguang Huo <zhuo@ufl.edu>
biocViews: StatisticalMethod, Software
VignetteBuilder: knitr
Description: Implementation by Zhiguang Huo
License: GPL-3
Depends: R (>= 3.5), tightClust
Imports: edgeR, limma, stats
BugReports: https://github.com/Caleb-Huo/AWFisher/issues
Suggests: knitr
Packaged: 2019-05-23 01:12:24 UTC; caleb
RoxygenNote: 6.0.1
NeedsCompilation: no

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.

mtmorgan commented 5 years ago

It is not appropriate to use data from github for your vignette, because this data may have a life span different from your package. Instead, if the data is or can be made sufficiently small, include it in your package. If the data is large, create an 'ExperimentData' package with the data stored in ExperimentHub.

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:

d343c20 Version: 0.99.0 to Version: 0.99.1

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.

Caleb-Huo commented 5 years ago

Dear Bioconductor team (@bioc-issue-bot @mtmorgan ):

Many thanks for your quick response!

I have addressed the following issues. 1, Add SSH keys to your GitHub account, following the instruction. 2, Add web hook, following the instruction. 3, Push my commit with message "Version: 0.99.0 to Version: 0.99.1" 4, Imbed my example data in the package instead of reading from an external source. 5, Fix the ERROR and WARNINGS in the build report.

Let me know if there are any other issues. Many thanks for your help!

Kayla-Morrell commented 5 years ago

Hi @Caleb-Huo,

Thank you for your submission to Bioconductor. Please see the initial review of 'AWFisher' below. Comment back here with updates that have been made and when the package is ready for re-review.

General package development

R BiocCheck

[ ] SUGGESTION: Consider shorter lines, 55 lines (6%) are > 80 characters long.
[ ] SUGGESTION: Consider 4 spaces instead of tabs; 189 lines (21%) contain tabs.
[ ] SUGGESTION: Consider multiples of 4 spaces for line indents, 14 lines (2%) are not.
[ ] CLARIFICATION: The raw package directory should not contain unnecessary files, what is the purpose of 'importanceSampling'?

DESCRIPTION

[ ] REQUIRED: The description should be a relatively short but detailed overview of what the package functionality entails. It should be one or more complete sentences.
[ ] REQUIRED: Since you are using a standard license you do not need to include the LICENSE file in your package.
[ ] REQUIRED: The package 'tightClust' in the 'Depends' field is not imported or imported from. It only seems to be used in the vignette file, so if this is the case it should be included in the 'Suggest:' field.

NAMESPACE

[ ] REQUIRED: Exported functions should use camel case or underscoring and not include “.” which indicates S3 dispatch.

NEWS

[ ] SUGGESTION: Consider adding a NEWS file so your package news will be included in the Bioconductor release announcements. The following url can help with formatting https://bioconductor.org/developers/package-guidelines/#news.

vignette

[ ] REQUIRED: Add a bit more detail to the 'Introduction' about introducing the objective, models, unique functions, key points, etc that distinguish the package from other packages of similar type (almost like an abstract).
[ ] REQUIRED: The 'Installation' section should show the users how to download and load the package from Bioconductor.
[ ] SUGGESTION: We strongly encourage the use of a table of contents.
[ ] REQUIRED: Line 61-63, I don't believe this is true (and therefore not needed) because the data is part of the package.
[ ] REQUIRED: Line 100 should not be commented out.
[ ] REQUIRED: Line 138 and 139 are not needed.
[ ] REQUIRED: Line 167 should be split into two lines since it is so long.
[ ] CLARIFICATION: When running through the code on my own, I notice that for lines 171 and 172 I get a different answer than what is shown in the vignette. Is this expected? If so you might mention somewhere in the comments that the answers produced can differ each time the funtion is run.
[ ] REQUIRED: The final section of the vignette should be 'Session Info' which includes SessionInfo().

man

[ ] REQUIRED: The authors tag in the man pages should include a full name, not just the first name.
[ ] CLARIFICATION: The man page 'function_edgeR' talks about using the limma package, which looks the same for 'function_limma'. Should this man page talk about 'edgeR' instead?

unit tests

[ ] SUGGESTION: Consider adding unit tests. We strongly encourage them.

R code

[ ] CLARIFICATION: What is the purpose of 'sysdata.rda' in the R directory?
[ ] REQUIRED: Avoid 1:...; use seq_len() or seq_along(). This is found in
- biomarkerCategorization (line 73 and 92)
- clusteringUtil (line 3, 8, 20, 24, and 47)
- variabilityIndex (line 51 and 64)
[ ] REQUIRED: Use various apply functions (vapply() over sapply()) instead of for loops were applicable.

Best, Kayla

bioc-issue-bot commented 5 years ago

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

909978c save changes 1 184d137 Version: 0.99.1 to Version: 0.99.2 d31b180 Version: 0.99.1 to Version: 0.99.2

Caleb-Huo commented 5 years ago

Hi @Kayla-Morrell

Thank you so much for your time and effort for this careful review. This is my first time to submit a package, so your suggestions are very helpful and instructive. I have incorporate majority of your suggestions in to my package. Please see below for my response to each of your suggestions. Any further comments/suggestions will be appreciated!

General package development

R BiocCheck_

[] SUGGESTION: Consider shorter lines, 55 lines (6%) are > 80 characters long. Have split long lines. However, some lines are hard to split. E.g., "@source \url{https://projecteuclid.org/download/pdfview_1/euclid.aoas/1310562214}" [] SUGGESTION: Consider 4 spaces instead of tabs; 189 lines (21%) contain tabs. Have used tidy_dir("R") to clean this up. [] SUGGESTION: Consider multiples of 4 spaces for line indents, 14 lines (2%) are not. Have used tidy_dir("R") to clean this up. [X] CLARIFICATION: The raw package directory should not contain unnecessary files, what is the purpose of 'importanceSampling'? Have removed this folder

DESCRIPTION

[X] REQUIRED: The description should be a relatively short but detailed overview of what the package functionality entails. It should be one or more complete sentences. Have revised [X] REQUIRED: Since you are using a standard license you do not need to include the LICENSE file in your package. Have revised [X] REQUIRED: The package 'tightClust' in the 'Depends' field is not imported or imported from. It only seems to be used in the vignette file, so if this is the case it should be included in the 'Suggest:' field. Have revised

NAMESPACE

[X] REQUIRED: Exported functions should use camel case or underscoring and not include “.” which indicates S3 dispatch. Have revised

NEWS

[X] SUGGESTION: Consider adding a NEWS file so your package news will be included in the Bioconductor release announcements. The following url can help with formatting https://bioconductor.org/developers/package-guidelines/#news. Have revised

vignette

[X] REQUIRED: Add a bit more detail to the 'Introduction' about introducing the objective, models, unique functions, key points, etc that distinguish the package from other packages of similar type (almost like an abstract). Have revised [X] REQUIRED: The 'Installation' section should show the users how to download and load the package from Bioconductor. Have revised [] SUGGESTION: We strongly encourage the use of a table of contents. Should have included a table of content in the vignette. [X] REQUIRED: Line 61-63, I don't believe this is true (and therefore not needed) because the data is part of the package. Have removed these two lines. [X] REQUIRED: Line 100 should not be commented out. Have revised [X] REQUIRED: Line 138 and 139 are not needed. Have revised [X] REQUIRED: Line 167 should be split into two lines since it is so long. Have revised [X] CLARIFICATION: When running through the code on my own, I notice that for lines 171 and 172 I get a different answer than what is shown in the vignette. Is this expected? If so you might mention somewhere in the comments that the answers produced can differ each time the function is run. Have set random seed to guarantee reproducibility. [X] REQUIRED: The final section of the vignette should be 'Session Info' which includes SessionInfo(). Have revised

man

[X] REQUIRED: The authors tag in the man pages should include a full name, not just the first name. Have revised [X] CLARIFICATION: The man page 'function_edgeR' talks about using the limma package, which looks the same for 'function_limma'. Should this man page talk about 'edgeR' instead? You are right. Have revised.

unit tests

[ ] SUGGESTION: Consider adding unit tests. We strongly encourage them.

R code

[X] CLARIFICATION: What is the purpose of 'sysdata.rda' in the R directory? This package uses importance sampling technique (very time consuming > 1 days) to generate a library (sysdata.rda). Then users can just call this library (sysdata.rda) to do fast computing (< 1 second). I have remove the importance sampling folder, which is the code how to generate the library. sysdata.rda is necessary in order to acheive fast computing. [X] REQUIRED: Avoid 1:...; use seq_len() or seq_along(). This is found in

biomarkerCategorization (line 73 and 92)
clusteringUtil (line 3, 8, 20, 24, and 47)
variabilityIndex (line 51 and 64)

Have revised

[ ] REQUIRED: Use various apply functions (vapply() over sapply()) instead of for loops were applicable. All for loops in my packages are used for resampling/permutation (procedure). Since the major computing part is about the procedure/iteration, instead of the iteration itself, I believe the improvement of vapply/sapply are really minimum. For example, the procedure will take 1min, while the iteration itself will only take 0.001 second. On the other hand, since the resampling/permutation procedures themselves are very long, so using vapply/sapply may make the code less interpretable. Therefore, I would prefer to stick with for loops.

Best, Caleb

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

@Caleb-Huo - Everything looks good. I have just one comment to make. I didn't realize that the folder 'importanceSampling' was the documentation/code for the 'sysdata.rda' file. Since you include the 'sysdata.rda' file in your package we prefer there to be documentation of it somewhere within the package. I would suggest adding the 'importanceSampling' folder back into your repo but include it in an inst/script/ directory. Once you add this folder back in and get a clean build then I'll be ready to accept the package.

Thank you, Kayla

bioc-issue-bot commented 5 years ago

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

6fe4076 0.99.2 to Version: 0.99.3

Caleb-Huo commented 5 years ago

@Kayla-Morrell I have put the 'importanceSampling' folder back into the repo and include it in the inst/script/ directory. In addition, I also add a readMe.txt file in that directory to provide instructions how to execute the codes and where to find the description of the algorithm.

Thanks again for your review!

Caleb

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: "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:

2198e00 0.99.3 to Version: 0.99.4, also fix a bug (empty i...

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 @Caleb-Huo,

Thank you for making that change, everything looks good now and I'm happy to accept the package!

Best, Kayla

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!

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

https://bioconductor.org/packages/AWFisher

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

AWFisher package submission to Bioconductor #1130

General package development

DESCRIPTION

NAMESPACE

NEWS

vignette

man

unit tests

R code

General package development

DESCRIPTION

NAMESPACE

NEWS

vignette

man

unit tests

R code