Closed lujunyan1118 closed 2 years ago
Hi @lujunyan1118
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: DepInfeR
Type: Package
Title: Inferring tumor-specific cancer dependencies through integrating ex-vivo drug response assays and drug-protein profiling
Version: 0.99.0
Authors@R:
c(person(given = "Junyan",
family = "Lu",
role = c("aut", "cre"),
email = "jylu1118@gmail.com",
comment = c(ORCID = "0000-0002-9211-0746")),
person(given = "Alina",
family = "Batzilla",
role = c("aut")))
Description: DepInfeR is an R package for a computational method that integrates two experimentally accessible input data matrices: the drug sensitivity profiles of cancer cell lines or primary tumors ex-vivo (X), and the drug affinities of a set of proteins (Y), to infer a matrix of molecular protein dependencies of the cancers (ß). DepInfeR deconvolves the protein inhibition effect on the viability phenotype by using regularized multivariate linear regression. It assigns an “dependence coefficient” to each protein and each sample, and therefore could be used to gain causal and accurate understanding of functional consequences of genomic aberrations in a heterogeneous disease, as well as to guide the choice of pharmacological intervention for a specific cancer type, sub-type, or an individual patient. For more information, please read out preprint on bioRxiv: https://doi.org/10.1101/2022.01.11.475864
License: GPL-3
Encoding: UTF-8
Imports:
matrixStats,
foreach,
doParallel,
glmnet,
doRNG,
parallel,
rlist,
tibble,
dplyr
Suggests:
testthat (>= 3.0.0),
ggplot2,
knitr,
rmarkdown,
tidyr,
tidyverse,
missForest,
pheatmap,
RColorBrewer,
ggrepel,
factoextra,
fpc,
ggbeeswarm,
gt,
DESeq2
VignetteBuilder: knitr
RoxygenNote: 7.1.2
biocViews: Software, Regression, Pharmacogenetics, Pharmacogenomics, FunctionalGenomics
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
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: 923851a1340a6f7230bf892a29ac5825a5583bbf
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @lujunyan1118,
Thank you for your submission to Bioconductor and thank you making the changes to get the 'OK' label added by the bot.
I will provide an initial review within 2-3 weeks (I have another package to review first).
In the meantime, please try to address the NOTE
s in the build report, as this will likely be part of my review.
Thanks, Pete
Received a valid push on git.bioconductor.org; starting a build for commit id: 007fe94c6801a3bf2b26d4ce09e56913a991299b
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Hi @lujunyan1118,
Thank you for your submission to Bioconductor. Overall, the package is in good shape. For acceptance into Bioconductor, I have a few Required points, as well as some Recommended points, that I would ask you to first please address.
Cheers, Pete
?runLASSOregression
, TargetMatrix Pre-processed drug-protein affinity matrix.
might be expanded to include what should be in the rows and columns of the matrix.BugReports
field to the DESCRIPTION
. This usually points to the package's GitHub issues page.if (!require("BiocManager", quietly = TRUE))
install.packages("BiocManager")
BiocManager::install("DepInfeR")
Depends: R (>= 4.2.0)
to DESCRIPTION
since your package will become part of Bioconductor version 3.15 which will depend on R 4.2.0.styler::style_pkg(filetype = "Rmd")
(perhaps specifying additional arguments) to yield a consistent style for the vignette. In my experience the output of styler::style_pkg()
may require some manual tweaks.Suggests
. Right now, it may be more effort than it's worth to reduce the reliance on so many depenencies but in the long term it may make maintenance easier to have fewer depenendices.README
(recommend README.md
so that it renders nicely on GitHub); see https://contributions.bioconductor.org/readme.htmlinst/CITATION
file so that people know what to cite when using your package.runLASSOregression()
to runLASSORegression()
@importFrom pkg fun
, e.g., @importFrom matixStats colSums2
. This may also reveal unnecessary depenencies.inst/extdata/
directory.
inst/extdata
relate to files in data
? In particular, the former don't appear to be directly used in the package so may be redundant.covr::report()
to see which lines are being tested by the unit tests and adding tests for any important parts that are not currently covered.select()
statement is made redundant by the second in in these lines.useTar <- rowSums(result$coefMat != 0) > 0
or similar? It could happen that the rowSum is zero but the elements are non-zero (yes, it's a pathological scenario).mutation_GDSC$TCGA.classification
is not yet a factor so setting the levels()
is probably not doing what you intend. These lines and those surrounding might need a review.NA
values in mutation_GDSC$TCGA.classification
because LAML
does not exist in vecor (AML
does, however, so I think this could be a type).Hi @PeteHaitch, thanks a lot for your effort in reviewing our package. The comments are very helpful and we will address those issues.
Best,
Junyan
Hi @lujunyan1118,
How are you going with addressing the review comments? We like to see some activity on the issue within 3 weeks. It's no problem if it's taking longer, but we will close the issue and you can re-open it when you are ready.
Cheers, Pete
Hi @PeteHaitch , I am still working on it. Hopefully I can submit it again next week.
Best,
Junyan
That's totally fine, thanks for the update :)
Received a valid push on git.bioconductor.org; starting a build for commit id: 099026966c2adc0d2d1889d1b1e12f873b9d1b0d
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: 5eafd088c6ef238938a9cdf22e8714f6ae3e67f0
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: 9c75717cc193a41630b2a98409b52b076d656b0e
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Received a valid push on git.bioconductor.org; starting a build for commit id: e2512db492800b5af362c6f2065f6a2b5ef9a70f
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Dear @PeteHaitch
Thanks a lot again for your very helpful comments on our package. We have updated our package to address all the “Required” and “Recommended” changes from you, except for your first comment about the possibility of using PharmacoSet object from PharmacoGx package. We explored this possibility. However, we still think our current solution of using a simple data matrix to store the drug response data fits our purpose the best. We don’t see any potential benefits from the additional features provided by the PharmacoSet structure. In addition, in the documentation of the PharmacoSet class, the authors wrote “We do not recommend creating PharmacoSet objects by the end-users, rather we recommend contacting us to have the objects created for your dataset of interest.”. So we believe this class is only intended to be used with their “PharmacoGx” package, and not a general data structure to store drug response data. Let me know if you have further comments or requests.
Best regards,
Junyan
Hi Junyan,
Thank you for updating the package in light of the initial review. Your rationale for not using PharmacoSet makes sense to me.
Would you please provide line-by-line comments to my initial review so that I know what changes I'm looking for in my re-review (sorry that I forgot to make this clear). I will then be able to finish the review this week and we should have the package accepted, soon.
Cheers, Pete
Hi Peter @PeteHaitch ,
Please see below for the line-by-line response to your initial comments. Please let me know if further modifications are needed. Thanks again for your effort!
Response: Thanks for the suggestion. We explored this possibility. However, we still think our current solution of using a simple data matrix to store the drug response data fits our purpose the best. We don’t see any potential benefits from the additional features provided by the PharmacoSet structure. In addition, in the documentation of the PharmacoSet class, the authors wrote “We do not recommend creating PharmacoSet objects by the end-users, rather we recommend contacting us to have the objects created for your dataset of interest.”. So we believe this class is only intended to be used with their “PharmacoGx” package, and not a general data structure to store drug response data.As mentioned in the previous response, we would like to stick to the simple data matrix for storing drug response data.
Response: We have expanded the documentation of those functions as suggested.
Response: We have checked the spell and corrected typos and grammatical errors.
Response: We added a legend below each figure in the vignette to briefly describe the content of the figure.
Response: Now we are using BiocParallel for parallelization in our package.
Response: We added “BugReports: https://github.com/Huber-group-EMBL/DepInfeR/issues” to the DESCRIPTION file.
BiocManager::install("DepInfeR")
Response: We added an installation section as shown above to the package vignette.
Response: We added “Depends: R (>= 4.2.0)” in the DESCRIPTION file.
Response: We updated the code formatting in the vignette by using styler::style_pkg() and manual tweaking.
Response: We have rewritten the two core functions to only use base R function and therefore tidyverse or dplyr is removed from the “Imports” field in DESCRIPTION. We also removed the dependency of some other unused packages, including “factoextra”, “rlang” and “rlist”
Response: This is because the input drug response matrix is a viability matrix and a high value represents actually weaker response (a drug kills less cells). However, we would like the output of our algorithm to directly indicate the dependency of a protein, which means if the protein targeted by the drug is important, the cells should have lower viability (stronger response) after the treatment of the drug. So, the protein dependency score outputted by our algorithm should have the opposite sign of the coefficient from the linear model. We have added a comment in the code.
Response: As suggested, we have broken the big chunks in this section (now section 8) into smaller ones and also added some details about the motivation, methods and interpretation in each chunk.
Response: We have modified those figures (now in section 8.1 and 8.3) to make the labels and texts more readable.
Response: The commented-out lines in the vignette are removed.
Response: The header formatting issues have been fixed.
Response: We added a README file as suggested in the GitHub.
Response: We highlighted our pre-print in the vignette, README and the documentation for the runLASSORegression() function. We also added an inst/CITATION file to tell users how to cite our package.
Response: We renamed this function as suggested.
Response: We removed unnecessary imports. For the matrixStats package, we are now using @importFrom matrixStats rowMedians
Response: Thanks for noticing this. Indeed the extdata is not necessary and we removed this folder. We also added more detailed documentation about the data sets in the R/dataset.R file.
Response: We added a package-level documentation at the beginning of the R/DepInfeR.R file, which instructs Roxygen to produce a DepInfeR-package.Rd file in man folder.
Response: We added sanity checks for arguments of the two main functions in the R/DepInfeR.R file.
Response: We updated the unit tests and now the functions are 100% covered according to covr::report()
Response: Thanks for noticing this. This select() is not necessary and we removed it in the vignette.
Response: We updated this plot in the vignette and added an explanation regarding the choice of the cut-off below the plot, which is “in order to keep as many cell lines as possible while reducing the missing data points, we chose one of the elbow points (x=24, shown as the red dashed line) in the plot above as the cut-off for allowed missing values. We will impute the remaining missing values by using the MissForest imputation method.”
Response: Thanks for the suggestion. We have changed this line to “useTar <- rowSums(result$coefMat != 0) > 0” as suggested.
Response: Thanks for noticing this. Indeed the levels() function here is not necessary and we removed it in the updated version.
Response: Yes, it is a typo. Thanks for noticing this. We have change “LAML” to “AML” and the NA issues have been fixed.
Best,
Junyan
Hi @lujunyan1118,
Thank you for addressing the comments in the initial review. I have a few additional comments and suggestions before acceptance.
Cheers, Pete
BPPARAM = bpparam()
argument to the function (see 'For Developers' section in https://bioconductor.org/packages/release/bioc/vignettes/BiocParallel/inst/doc/Introduction_To_BiocParallel.pdf). I'll ask some experts (@mtmorgan or @vjcitn) to please comment if the current implementation, which exposes cores
and RNGseed
parameters and then constructs a MultiCoreParam (https://github.com/Huber-group-EMBL/DepInfeR/blob/e2512db492800b5af362c6f2065f6a2b5ef9a70f/R/depInfeR.R#L169-L191), is appropriate/satisfactory?gsub()
).library()
calls in vignette need to be, e.g., A user doesn't need library(glmnet)
nor library(BiocParallel)
to run the vignette code.library()
statements to the code chunk that first uses it (e.g., ggrepel isn't needed until Section 8.4, so perhaps don't load it until then so that it's easier for the reader to understand that it is not needed for the earlier parts of the vignette).fig.cap
for proper figure captions (see https://yihui.org/knitr/options/#plots) rather than just having these as a paragraph of text appearing after the image.\url{}
around URLs in documentation so that they are hyperlinks.filter()
and dplyr::filter()
are both used).Received a valid push on git.bioconductor.org; starting a build for commit id: 30848ac390ea1aeb911d93081420a02481858113
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/DepInfeR
to trigger a new build.
A quick tutorial for setting up remotes and pushing to upstream can be found here.
Dear Peter @PeteHaitch ,
Thanks very much for the additional comments. We have updated our package to include those suggestions. Please see the line-by-line response below. Please let me know if further modifications are needed.
Required Usage of BiocParallel is typically done by providing a BPPARAM = bpparam() argument to the function (see 'For Developers' section in https://bioconductor.org/packages/release/bioc/vignettes/BiocParallel/inst/doc/Introduction_To_BiocParallel.pdf). I'll ask some experts (@mtmorgan or @vjcitn) to please comment if the current implementation, which exposes cores and RNGseed parameters and then constructs a MultiCoreParam (https://github.com/Huber-group-EMBL/DepInfeR/blob/e2512db492800b5af362c6f2065f6a2b5ef9a70f/R/depInfeR.R#L169-L191), is appropriate/satisfactory?
Response: As suggested, we have changed the BiocParallel implementation according to the BiocParallel package documentation. The users now need to construct their own parallel back-end and pass it to the runLASSORegression function through the BPPARAM argument, otherwise, the default back-end will be used. I haven’t heard from Martin or Vincent regarding this issue. Please let me know if the implementation is acceptable now.
Recommended
The vignette still uses a number of large-ish dependencies; consider further reducing these. E.g., I don't think you need the full tidyverse but perhaps 'just' tidyr, dplyr, and stringr (and I think this could be replaced with appropriate calls to gsub()). Response: We have changed the dependence on tidyverse to the individual packages of tibble, tidyr and dplyr. We now use gsub() instead of str_replace() so that stringer dependence is not required.
Not all packages loaded/attached with library() calls in vignette need to be, e.g., A user doesn't need library(glmnet) nor library(BiocParallel) to run the vignette code. Response: We removed “library(glmnet)” in the vignette. But we kept “library(BiocParallel)”. This is because we modified the runLASSORegression() function to use the BPPARAM = bpparam() argument as mentioned above. So to specify the random generator seed, we need to manually construct the back-end using MulticoreParam() function from “BiocParallel” package.
Consider moving library() statements to the code chunk that first uses it (e.g., ggrepel isn't needed until Section 8.4, so perhaps don't load it until then so that it's easier for the reader to understand that it is not needed for the earlier parts of the vignette). Response: As suggested, we have moved the loading of the packages, including ggrepel, pheatmap, RColorBrewer and ggbeeswarm to the chunks where they were firstly used.
Consider using fig.cap for proper figure captions (see https://yihui.org/knitr/options/#plots) rather than just having these as a paragraph of text appearing after the image. Response: We are now using the fig.cap option for showing the figure captions in the vignette.
Consider using \url{} around URLs in documentation so that they are hyperlinks. Response: We are now using \url{} for the urls in the package documentation.
Try to be consistent with coding style in vignette (e.g., filter() and dplyr::filter() are both used). Response: We have made the coding style consistent in vignette. For example, all the dplyr::filter() and dplyr::select() are changed to filter() and select(), respectively.
Your package has been accepted. It will be added to the Bioconductor nightly builds.
Thank you for contributing to Bioconductor!
Reviewers for Bioconductor packages are volunteers from the Bioconductor community. If you are interested in becoming a Bioconductor package reviewer, please see Reviewers Expectations.
Thank you for your responses, @lujunyan1118. I'm pleased to accept DepInfeR into Bioconductor 🎉 Thank you for your contribution!
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/lujunyan1118.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("DepInfeR")
. The package 'landing page' will be created at
https://bioconductor.org/packages/DepInfeR
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 Bioconductor Package Naming Policy and acknowledge Bioconductor may retain use of package name.
[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.
[X] I am familiar with the Bioconductor code of conduct and agree to abide by it.
I am familiar with the essential aspects of Bioconductor software management, including:
For questions/help about the submission process, including questions about the output of the automatic reports generated by the SPB (Single Package Builder), please use the #package-submission channel of our Community Slack. Follow the link on the home page of the Bioconductor website to sign up.