search

Bioconductor / Contributions

Contribute Packages to Bioconductor
134 stars 33 forks source link

(inactive) ComStat #923

Closed gzrrobert closed 5 years ago

gzrrobert 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 @gzrrobert

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: ComStat
Title: Design & Analysis of Synergy Experiment
Version: 0.0.1.0000
Author: Zhangrong Gu,  Ruijie Yin,  Hongbin Fang,  Ming Tan
Maintainer: Zhangrong Gu <gzrrobert@gmail.com>
Description: Substituting the Hill model of each drug with linear/log-linear model, 
    designing a dose-mixture scheme for combination study,
    implementing an F-test to find out the additive/synergistic/antagonistic effect under each combination, 
    fitting a combination index surface and drawing a contour plot. 
    Citationsare are Tan (2003) <10.1002/sim.1467>,
 Fang (2008) <10.1002/sim.3204>,
           Fang (2009) <10.1080/10543400902964019>,
           Tan (2015) <10.1177/0962280215574320>.
Contact: Zhangrong Gu <gzrrobert@gmail.com> Ming Tan
        <mtt34@georgetown.edu> Hongbin Fang <hf183@georgetown.edu>
Depends: R (>= 3.3.1)
Imports: BB, ktsolve, nleqslv, drc, fields, Matrix, splines,
        scatterplot3d
License: MIT + file LICENSE
Encoding: UTF-8
LazyData: true
RoxygenNote: 6.0.1
NeedsCompilation: no
Packaged: 2018-06-22 14:18:36 UTC; Robert Gu
mtmorgan commented 5 years ago

Please review the Bioconductor package guidelines. Your package does not have a vignette and does not interoperate with robust, established packages relevant to the domain of analysis. Is it more appropriate as a CRAN package?

gzrrobert commented 5 years ago

The package is about the statistical design for drug combination and draw the surface and contour plot, so I think it would be proper to GitHub.

mtmorgan commented 5 years ago

'Proper to GitHub'? Then you do not intend to submit it to CRAN or Bioconductor? If you do wish it to be considered for Bioconductor, it must have a vignette and should use Bioconductor infrastructure packages, as appropriate, for interoperability.

gzrrobert commented 5 years ago

Sorry, I am intending it to Bioconductor, but what is a vignette?

mtmorgan commented 5 years ago

A vignette is a document that describes how the package is used, typically with a working example and emphasizing integration with other Bioconductor packages.

An excellent vignette is from DESeq2 (from the source in the package /vignettes directory -- git clone https://git.bioconductor.org/packages/DESeq2)

Some basic instructions for producing a vignette are here

gzrrobert commented 5 years ago

Okay, thank you so much about the instruction, but I am still confused about the infrastructure packages http://bioconductor.org/developers/how-to/efficient-code/ instruction.

Best regards, Zhangrong Gu, MS in Biostatistics Research Assistant Department of Biostatistics, Bioinformatics & Biomathematics Georgetown University Medical Center +1-(202) 294-2414

Martin Morgan notifications@github.com 于2018年10月30日周二 下午11:30写道：

A vignette is a document that describes how the package is used, typically with a working example and emphasizing integration with other Bioconductor packages.

An excellent vignette is from DESeq2 http://bioconductor.org/packages/devel/bioc/vignettes/DESeq2/inst/doc/DESeq2.html (from the source in the package /vignettes directory -- git clone https://git.bioconductor.org/packages/DESeq2)

Some basic instructions for producing a vignette are here http://r-pkgs.had.co.nz/vignettes.html

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

mtmorgan commented 5 years ago

what are the main inputs and outputs of your functions?

mtmorgan commented 5 years ago

Several problems immediately appear to me in your code.

The first is that examples cannot refer to locations on your disk, since our build system (that runs the examples) does not have access to your disk. Instead, put example data sets in inst/extdata/<your-data-here> and use the system.file() function in examples / vignettes to locate these -- system.file(package="ComSat", "extdata", "<your-data-here")

The second is that users will often have read their data into R's memory and manipulated the objects themselves, so they should be able to pass a data.frame or similar object rather than file paths.

The third is that R operates on vectors, so loops such as the one at https://github.com/gzrrobert/ComStat/blob/b949bd076659a983662c8997c27cd0b37b8bb5d2/R/TwoDrug1.R#L163

  for (i in 1:length(linear_hat)) {
    diff_linear_hill[i]<-(linear_hat[i]-hill_hat[i])**2
  }

are much more efficiently written as simply

 diff_linear_hill <- (linear_hat - hill_hat) ** 2

This applies at many places in your code, in this 'obvious' way and in more subtle ways.

Using <<- at https://github.com/gzrrobert/ComStat/blob/b949bd076659a983662c8997c27cd0b37b8bb5d2/R/TwoDrug1.R#L49 will over-write any variable on the left-hand side that the user has in their global environment; this is very bad practice and should be avoided -- your code should not rely on a variable "location1" that is present in the user's global environment, but should instead rely on arguments passed to the function.

gzrrobert commented 5 years ago

Okay, I see the problems, thank you so much for your help, and I have uploaded the data used in the example. I will also change the location in the example so that the users could download the example data from Github and put their own path on their disk to run the example.

mtmorgan commented 5 years ago

the data is small and should be included in the package (in the inst/extdata folder mentioned previously). The user should not have to separately download it from github.

gzrrobert commented 5 years ago

Okay, sure!

mtmorgan commented 5 years ago

Can you update me on the status of your package, have you addressed the data and other issues so that your package is ready for moderation and addition to the review queue?

gzrrobert commented 5 years ago

Hi Martin, I have included the data in the package.

Best regards, Zhangrong Gu, MS in Biostatistics Research Assistant Department of Biostatistics, Bioinformatics & Biomathematics Georgetown University Medical Center +1-(202) 294-2414

Martin Morgan notifications@github.com 于2018年11月21日周三 上午12:37写道：

Can you update me on the status of your package, have you addressed the data and other issues so that your package is ready for moderation and addition to the review queue?

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

mtmorgan commented 5 years ago

your code still contains loops that can be easily vectorized. There is no vignette in your package. I think the effort required to bring this to Bioconductor standards is substantial, and I am therefore declining the package.