search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

MAGeCKFlute #509

Closed WubingZhang closed 6 years ago

WubingZhang commented 6 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 6 years ago

Hi @WubingZhang

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: MAGeCKFlute
Type: Package
Title: Functional analytical pipeline for CRISPR screens based on MAGeCK's gene summary results
Version: 0.1.0
Date: 2017-08-24
Author: Wubing Zhang, Feizhen Wu
Maintainer: Wubing Zhang<Watson5bZhang@gmail.com>
Description: This MAGeCKFlute package provides a functional analytical pipeline for CRISPR screening dataset.
License: GPL (>=3)
Depends: R (>= 3.2.1), ggplot2 (>= 2.1.0), ggExtra (>= 0.3), gridExtra (>= 2.0.1), affy (>= 1.40.0), fitdistrplus (>= 1.0.0), reshape (>= 0.6.3), ggsci (>= 1.0), ggrepel (>= 0.3.5), grid (>= 3.1.0), org.Hs.eg.db (>= 2.1.0), org.Mm.eg.db (>= 2.1.0), pathview (>= 1.6.0), Category (>= 2.20.0), GOstats (>= 2.20.0), GO.db (>= 2.1.0), png (>= 0.1.0), clusterProfiler (>= 2.0.0), GSEABase (>= 1.20.0)
LazyData: TRUE
RoxygenNote: 6.0.1
mtmorgan commented 6 years ago

Bioconductor vignettes must have fully evaluated code chunks, e.g., .Rnw files with <<>>=.

WubingZhang commented 6 years ago

Thanks, I'll add regular vignettes soon.

On Wed, Oct 4, 2017 at 9:53 PM, Martin Morgan notifications@github.com wrote:

Bioconductor vignettes must have fully evaluated code chunks, e.g., .Rnw files with <<>>=.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/509#issuecomment-334162557, or mute the thread https://github.com/notifications/unsubscribe-auth/AeZxre5EkhhTssc3L4bxQ7ikjh8K48Z9ks5so43rgaJpZM4PtkcV .

-- Watson Wubing Zhang Shirley Liu lab in Tongji University Biostatistics and Computational Biology

WubingZhang commented 6 years ago

Hi,

I have added regular vignettes. Please check again.

Thanks.

On Wed, Oct 4, 2017 at 10:05 PM, Watson Zhang watson5bzhang@gmail.com wrote:

Thanks, I'll add regular vignettes soon.

On Wed, Oct 4, 2017 at 9:53 PM, Martin Morgan notifications@github.com wrote:

Bioconductor vignettes must have fully evaluated code chunks, e.g., .Rnw files with <<>>=.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/509#issuecomment-334162557, or mute the thread https://github.com/notifications/unsubscribe-auth/AeZxre5EkhhTssc3L4bxQ7ikjh8K48Z9ks5so43rgaJpZM4PtkcV .

-- Watson Wubing Zhang Shirley Liu lab in Tongji University Biostatistics and Computational Biology

-- Watson Wubing Zhang Shirley Liu lab in Tongji University Biostatistics and Computational Biology

bioc-issue-bot commented 6 years ago

Your package has been approved for building. Your package is now submitted to our queue.

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 6 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, 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171005064804.html

bioc-issue-bot commented 6 years ago

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

38aed8f Remove all warnings

lawremi commented 6 years ago

The vignette should at least tell the user how to read the summary data when not using one of the included datasets.

WubingZhang commented 6 years ago

Hi Michael,

Thanks for your suggestion. I add 'Input data' section in the vignette, which should be clear to guide users to prepare the summary data. Please check it again.

Thanks one more time.

On Sat, Oct 7, 2017 at 5:14 AM, Michael Lawrence notifications@github.com wrote:

The vignette should at least tell the user how to read the summary data when not using one of the included datasets.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/509#issuecomment-334871821, or mute the thread https://github.com/notifications/unsubscribe-auth/AeZxrRiKsPBJz_F6ewY5IWhnUw10F3Jfks5sppgngaJpZM4PtkcV .

-- Watson Wubing Zhang Shirley Liu lab in Tongji University Biostatistics and Computational Biology

mtmorgan commented 6 years ago

remember to increment the version number to trigger a build of your package (to 0.99.1)

bioc-issue-bot commented 6 years ago

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

738a5c2 Update version to 0.99.1 and add new features

bioc-issue-bot commented 6 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". 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023062730.html

bioc-issue-bot commented 6 years ago

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

bd4ffb5 bump version

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023073326.html

bioc-issue-bot commented 6 years ago

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

94ff87c Add new runnable examples

bioc-issue-bot commented 6 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: "TIMEOUT". 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023084048.html

bioc-issue-bot commented 6 years ago

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

3b8e7e3 Deal with the problem of timeout in examples

bioc-issue-bot commented 6 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: "TIMEOUT, 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023094130.html

bioc-issue-bot commented 6 years ago

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

6137f04 Shorten time ac465b6 Shorten time

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023102943.html

bioc-issue-bot commented 6 years ago

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

4f040b1 Remove errors

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023104903.html

bioc-issue-bot commented 6 years ago

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

2fa5346 Remove errors and warnings

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023124708.html

bioc-issue-bot commented 6 years ago

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

80e34e0 Remove all errors and warnings 1eeb31a remove all errors and warnings

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023134446.html

bioc-issue-bot commented 6 years ago

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

d0e6a33 bump version

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171023140306.html

WubingZhang commented 6 years ago

Hi,

My package has been built without errors or warnings, should I do some other things or just wait?

Thanks!

mtmorgan commented 6 years ago

Wait; I will try to review your package today.

WubingZhang commented 6 years ago

Hi Martin, I have waited for about one week, and don't get any feedback. So can you help to check the status of my package? Thanks!

mtmorgan commented 6 years ago

DESCRIPTION

NAMESPACE

vignettes

R

man

bioc-issue-bot commented 6 years ago

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

3cfc7c1 change version to 1.1.0

bioc-issue-bot commented 6 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, 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171117125218.html

WubingZhang commented 6 years ago

Hi,

I revised my package according your suggestion, and push to Github. However, no response found. Could you please help me check why?

Many thanks.

On Sat, Nov 18, 2017 at 1:52 AM, bioc-issue-bot notifications@github.com wrote:

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, 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_ buildreport_20171117125218.html

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/509#issuecomment-345315326, or mute the thread https://github.com/notifications/unsubscribe-auth/AeZxrVDYc4UJ6z0ZI6aAWHXUu4Vcaswlks5s3cfUgaJpZM4PtkcV .

-- Watson Wubing Zhang Shirley Liu lab in Tongji University Biostatistics and Computational Biology

bioc-issue-bot commented 6 years ago

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

c4bf343 0.99.12

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171118061215.html

bioc-issue-bot commented 6 years ago

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

c69fd21 0.99.13

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171118075027.html

bioc-issue-bot commented 6 years ago

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

baab469 bump version

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171119201128.html

bioc-issue-bot commented 6 years ago

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

3976bff Depends on R version 3.5

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171119203230.html

WubingZhang commented 6 years ago

Hi,

I have revised my R package 'MAGeCKFlute' completely according to your suggestions. Could you please help to check again?

Many thanks!

On Thu, Nov 9, 2017 at 11:11 PM, Martin Morgan notifications@github.com wrote:

DESCRIPTION

  • Please elaborate on the Description:, so that users have a clear idea of what the package does.
  • Move packages that are needed for package function but not for end-user interaction to the Imports: field from the Depends: field.
  • Carefully consider the need for each package your package depends on -- with a large number of dependencies, your package becomes very fragile and subject to changes in these packages.

NAMESPACE

  • With so many packages, it makes sense to selectively import functions used by your package (importFrom()) rather than the entire package the except= argument to import() may also be helpful.

vignettes

-

Important functionality like EnrichAB() is not evaluated in the code. The commented-out example in the man page for this function looks like you are trying to 'game the system' by having a runnable example but not actually doing any work. There are no unit tests. There is no value in providing code that is not tested -- construct light-weight examples that are realistic but do not require extensive computation time, and use these examples in your examples and vignettes.

data("BRAF_mle.gene_summary") loads several objects into the global name space. The objects are poorly documented and of uncertain provenance. Elaborate in the help page on how each of the objects was obtained. Save the objects individually with saveRDS() an load with readRDS(), assigning to variables so that it is clear where symbols in the vignette come from.

The vignette says "To run MAGeCKFlute pipeline, we need gene summary file generated by running MAGeCK" but the user does not know what MAGeCK is, and does not know how to transform the output of MAGeCK to the inputs required here. Likely the vignette should start with importMAGeCK(), referencing output files from MAGeCK stored in the inst/extdata folder of the package.

BRAF_mle.gene_summary seems like it should be arranged as a SummarizedExperiment with assays beta, z, p.value, fdr, wald.p.value and waldd.fdr. rowData() would be Gene and sgRNA. This would ensure that all metrics are summarized for each sample, and that types were consistent within metric. The cntrlName and treatName should be stored as a factor in the colData() of the SummarizedExperiment.

The ReadBeta() function is named in a misleading way, implying that data is read from the disk.

Chatty output like

dd_loess = NormalizeBeta(dd, method="loess") 2017-11-07 10:01:58 # Normalize beta scores ... Done with 1 vs 2 in iteration 1 1 0.0001219035

quickly becomes tedious, especially for fast calculation. There should be a verbose = FALSE argument to each function.

Adopt a consistent plot function naming scheme, e.g., Violin.plot() and CellCycleFit() both seem to have primary goal the production of a plot.

R

  • The code needs to be re-factored so that numerical calculations are clearly separated from plotting functionality.
  • if() statements throughout the code lead to highly complicated code paths that are difficult to understand and test ('cyclomatic complexity'). The code should be re-factored to provide simple, distinct interfaces to the main work flows, each implemented with a minimum of conditional branching. Rather than tolerating many exceptions (e.g., presence or absence of particular arguments) provide appropriate defaults and start the function by asserting that inputs are consistent with the linear code path implemented in the function.
  • Use KEGGREST https://bioconductor.org/packages/KEGGREST and other existing packages to interact with online resources. Use BiocFileCache https://bioconductor.org/packages/BiocFileCache to save results that are likely to be used across sessions.
  • Use tempdir() or tempfile() rather than "." as the default directory / output file, so that the user does not accidentally overwrite their previous results.
  • eval(parse(text="")) is almost never needed; refactor code to avoid this.
  • don't install packages dynamically (getOrg.R:33) but instead tell the user to do so.
  • use requireNamespace() or loadNamespace() and resolve necessary symbols using pkg::fun() instead of library().
  • use seq_along() or seq_len() rather than 1:npath and similar, throughout.
  • KeggPathwayView.R:164: use warning() rather than message("Waring:...
  • loginfo.R: Logging info should be written to stderr (message()) rather than stdout (cat()).
  • ReadBeta.R:14 the stop() text is not accurate.
  • Always use drop=FALSE when subsetting columns of a data.frame.

man

  • describe data sets more completely, e.g., describing what each row represents, what columns are present, and what the columns represent.
  • Each example should evaluate the function being documented.
  • Use R variable naming conventions to identify the type of each argument. For instance enrich.DAVID.Rd describes gene as an 'interested gene list' but probably this is a character() vector. Be sure the type of each argument is clearly specified, e.g., 'idType' as character(1) gene id type for enrichment analysis in DAVID. Include possible or example values where appropriate.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/Bioconductor/Contributions/issues/509#issuecomment-343184092, or mute the thread https://github.com/notifications/unsubscribe-auth/AeZxrb4QUsDyEQ7KTYjUrHRSMUnCb1Hdks5s0xYggaJpZM4PtkcV .

-- Watson Wubing Zhang Shirley Liu lab in Tongji University Biostatistics and Computational Biology

bioc-issue-bot commented 6 years ago

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

33a0a9d 0.99.16

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171123083734.html

bioc-issue-bot commented 6 years ago

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

949eb6a 0.99.17

bioc-issue-bot commented 6 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 following build report for more details:

http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171123084759.html

WubingZhang commented 6 years ago

Dear reviewer, I have finished the revision for several days, so could you please check again? Many thansk! Monday, 20 November 2017, 09:32AM +08:00 from bioc-issue-bot notifications@github.com :

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 following build report for more details: http://bioconductor.org/spb_reports/MAGeCKFlute_buildreport_20171119203230.html — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub , or mute the thread .

bioc-issue-bot commented 6 years ago

Your package has been accepted. It will be added to the Bioconductor Git repository and nightly builds. Additional information will be sent to the maintainer email address in the next several days.

Thank you for contributing to Bioconductor!