Deep characterization of cancer drugs mechanism of action

[TrinhNguyenP]

Update the following URL to point to the GitHub repository of the package you wish to submit to Bioconductor

• Repository: https://github.com/CBIIT-CGBB/DeepTarget

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:

• [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 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.

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

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.

The following are build products from R CMD build on the Bioconductor Build System: Linux (Ubuntu 22.04.2 LTS): DeepTarget_0.99.10.tar.gz macOS 12.6.5 Monterey: DeepTarget_0.99.10.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[TrinhNguyenP]

"ERROR: At least 80% of man pages documenting exported objects must have runnable examples" this means that you have man pages without examples. You need to find which these are, and add an example(s) or explain why there isn't one.

Hi @PeteHaitch.

I have push an updated version and there still have three warnings. There is one warning that I can't locate. Can you please provide me some suggestion if possible? The warning is: WARNING Avoid T/F variables; IF logical, use TRUE/FALSE.

Thanks for your help so far.

Trinh

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 12803e4d4a3f253fd4be4bad5c88717b8cf55749

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Build System.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Bioconductor Build System: macOS 12.6.5 Monterey: DeepTarget_0.99.11.tar.gz Linux (Ubuntu 22.04.2 LTS): DeepTarget_0.99.11.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[TrinhNguyenP]

Hi @PeteHaitch, the Package now built without errors or warnings on all platforms. Thanks, Trinh

[PeteHaitch]

Hi @TrinhNguyenP,

Thank you for submitting DeepTarget to Bioconductor. I'm afraid that in its current state it is not suitable for Bioconductor and would require substantial changes before it could be accepted.

In my review below I have separated the issues into Required and Recommended points that I would ask you to address before the package can be accepted. 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.

Cheers, Pete

Required

• [ ] From where are the datasets originally available and documented/explained? The README says, "For full datasets used in the manuscript, please contact us at tinh.nguyen@nih.gov. For the detail of the links for downloading the data, please do ??DeepTarget." Why can't this information be included in the package instead of users having to email someone to get the full datasets? Furthermore, running ??DeepTarget doesn't tell a user anything useful; running ?OntargetM has some poorly formatted plain text links to files on the www.depmap.org but with no real context or explanation.
• [ ] Where's the full dataset available in this package? The DESCRIPTION says "This package spans about 1500 drugs across about 18K possible target genes" but the included data (OntargetM) is seemingly much smaller.

> str(OntargetM)
List of 5
 $ DrugMetadata   :'data.frame':    11 obs. of  5 variables:
  ..$ broad_id_trimmed: chr [1:11] "A27883417" "K02113016" "K09951645" "K38527262" ...
  ..$ name            : Factor w/ 4518 levels "1-((Z)-3-Chloroallyl)-1,3,5,7-tetraazaadamantan-1-ium",..: 250 2977 1213 462 3081 3537 3583 313 2065 3311 ...
  ..$ target          : Factor w/ 1830 levels "AADACL2","AADAT, AASS, ABAT, ALDH18A1, ASNS, BCAT1, BCAT2, CCBL2, DNPEP, EARS2, ENPEP, EPRS, FOLH1, FPGS, FTCD, GAD1, GAD"| __truncated__,..: 1648 1525 460 1308 604 1168 1399 1418 453 1174 ...
  ..$ drug_category   : Factor w/ 3 levels "chemo","noncancer",..: 3 3 3 3 3 3 3 3 3 2 ...
  ..$ moa             : Factor w/ 1072 levels "11-beta hydroxysteroid dehydrogenase inhibitor",..: 834 825 905 644 259 563 700 697 217 590 ...
 $ secondary_prism: num [1:16, 1:392] 0.864 1.335 NA 1.518 0.955 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:16] "A27883417" "K02113016" "K02113016" "K09951645" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...
 $ avana_CRISPR   : num [1:487, 1:392] 0.1191 0.0405 -0.1317 0.0816 -0.1112 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:487] "ABL2" "ACAT2" "ACCSL" "ACTA1" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...
 $ mutations_mat  : num [1:476, 1:392] 0 0 0 0 0 0 0 0 0 0 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:476] "ABL2" "ACAT2" "ACCSL" "ACTA1" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...
 $ expression_20Q4: num [1:550, 1:392] 6.13 2.12 7.12 1.35 6.46 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:550] "CD99" "TMEM176A" "POLDIP2" "USH1C" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...

• [ ] The vignette is inadequate. Some of the issues include:
  • There's not enough explanation of what the code examples are doing or why.
  • Outputs (e.g., plots and previews of data.frame objects) lack sufficient explanation or guidance on interpretation.
  • The vignette should include a detailed introduction/abstract section that provides motivation for inclusion in Bioconductor and when appropriate a review and comparison to existing Bioconductor packages with similar functionality or scope.
  • Random commented out lines, e.g., dir.create(), saveRDS().
• [ ] Please proofread the documentation and vignette, as well as run a spell checker on the package (if you use RStudio then there is one built in to help). Imagine a new user trying to use your package and make it as simple for them as possible by not distracting them with strange formatting and typos.
• [ ] For inclusion in Bioconductor, parallelisation should be with BiocParallel (e.g., use BiocParallel::bplapply() rather than parallel::mclapply() in DoPWY(); there are other uses of mclapply() that should also be changed); see https://contributions.bioconductor.org/r-code.html#parallel-recommendations. Furthermore, functions must not hard-code the parallelisation option (e.g., DoPWY() hardcodes mc.cores = 2 when computing DoPWYEnr).
• [ ] What is inst/ExWorkflow.R? Please remove it from the git repo or explain why it is required.
• [ ] Please use a more descriptive filename for the vignette than V_analyses.Rmd.
• [ ] The man pages are inadequate. Some of the issues include but are not limited to:
  • Function arguments are not adequately documented. E.g., ?DoPWY says that Sim.GES.DRS is the result of CorCRISPRDrugR(), a function that doesn't exist in DeepTarget.
  • GetSim() says it returns a data.frame but returns a list of matrix objects.
  • PredTarget() says it returns a table but returns a data.frame and it is not documented what the column names mean.
  • The examples for ?colorBar and ?line2user are very long and yet still inadequate because they don't clearly exemplify how/why to use these fucntions.
• [ ] Function names are uninformative.
  • E.g., According to the man page, GetSim() "Compute[s] a correlation between the every gene crispr KO vs each drug response". Leaving aside the explanation, which I don't understand, I can't see how 'GetSim' is a good name for this function.
  • Unexplained acronyms and abbreviations used throughout, e.g., DMB(), DTR() etc.
• [ ] The cor.test_trimmed_v0.default() function is a big red-flag that this package requires substantial revision before it would be suitable for Bioconductor.
  • It seems to have copy-pasted code from stats::cor.test(). Is this correct? Why is this function needed. You can't just copy+paste code without attribution (not to mention it being subject to the original code's licence).
  • Furthermore, it seems like cor.test_trimmed_v0.default() shouldn't be exported because it's documented as "This will be called inside other function such as Getsim".
• [ ] The errHandle() function is another red-flag.
  • It's documented as err_handle() and the documentation is uninformative.
  • It's basically tryCatch().
  • Why is this exported?
• [ ] The fdrCor() function is another red-flag.
  • The documentation is inadequate and doesn't credit p.adjust().
  • It's basically p.adjust().
  • Why is this exported?
• [ ] BiocCheck::BiocCheck() generates 15 NOTES, many of which are easily addressed and so should be.
• [ ] The README is poorly formatted and lacking structure. Please fix.
• [ ] The DESCRIPTION contains typos, incorrect information, and is hard to understand.
• [ ] In NAMESPACE, please use selective imports using importFrom instead of import all with import.
• [ ] Exported data and the data/ directory must be well documented.
  • OntargetM documentation is inadequate.
  • Says, "A list of 5 matrices" but it's actually a list of 1 data.frame and 4 matrix objects.
  • There's no explanation of column names.
  • Poor formatting makes it hard to read documention.
  • Documentation doesn't match data. The dimensions of objects don't match. E.g., nrow(OntargetM$avana_CRISPR) is 487 but documented as 488, nrow(OntargetM$mutations_mat) is 476 but documented as 470
• [ ] There's no obvious interoperability with other Bioconductor packages.
  • The data structures used in the package are ad hoc; are there existing Bioconductor data structures you could re-use?
  • Could you show/explain in the vignette how this package might be used in conjunction with other Bioconductor workflows and software

Recommended

• [ ] It is strongly recommended to add unit tests; see https://contributions.bioconductor.org/tests.html.
• [ ] Some functions return data.table objects (e.g., DoPWY()) and others a data.frame. It would be better to be consistent. This should be precisely documented in the 'Value' section of each man page.
• [ ] The example from plotSim() is poorly/weirdly formatted.
• [ ] There's no need to use print() of code comments in the vignette (e.g., print ( " we will focus on these drugs:");); a plain comment (e.g., # For the vignette we will focus on the following drugs) is sufficient.
• [ ] The code is quite hard to read and would benefit from following a consistent coding style. See https://contributions.bioconductor.org/r-code.html#coding-style for the Bioconductor code style guide, although this it not the only acceptable style. But code like S.Drug <- OntargetM$DrugMetadata$broad_id_trimmed [idx.Drug] is quite hard to read and makes its harder for a user to follow.
• [ ] Recommend omitting LazyData: true in DESCRIPTION; see https://contributions.bioconductor.org/description.html#description-lazydata.
• [ ] Consider adding a inst/CITATION; see https://contributions.bioconductor.org/citation.html.

[TrinhNguyenP]

Hi @PeteHaitch,

Thanks very much for your comments. I will respond to each of your comments and send them to you as soon as I am done.

Best,

Trinh

[PeteHaitch]

Hi @TrinhNguyenP ,

I'm checking to see if you intend to continue this submission? It's no problem if you're too busy at the moment, but please let us know if that's the case. If there's no response by the end of the week, I'll close this issue until you have further updates (we can re-open this issue and continue the submission if that's what you want to do).

Cheers, Pete

[TrinhNguyenP]

Good morning @PeteHaitch ,

Sorry for slow submission. We have made some good progress based on your required suggestions. We still have a few of them need to be addressed. I expect to have all done in one or two more weeks.

Thanks,

Trinh

[PeteHaitch]

No problem, @TrinhNguyenP. I'll close this issue for now, but you can re-open it by posting back here when you're ready to continue.

Cheers, Pete

[bioc-issue-bot]

This issue is being closed because there has been no progress for an extended period of time. You may reopen the issue when you have the time to actively participate in the review / submission process. Please also keep in mind that a package accepted to Bioconductor requires a commitment on your part to ongoing maintenance.

Thank you for your interest in Bioconductor.

[TrinhNguyenP]

No problem, @TrinhNguyenP. I'll close this issue for now, but you can re-open it by posting back here when you're ready to continue.

Cheers, Pete

Hi @PeteHaitch, I am ready to submit an update. Can you please let me know what should I do? Thanks, Trinh

[bioc-issue-bot]

Dear @TrinhNguyenP ,

We have reopened the issue to continue the review process. Please remember to push a version bump to git.bioconductor.org to trigger a new build.

[bioc-issue-bot]

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 8dff0e95801c86cdb96d0e87696e955a51d5a96a

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): DeepTarget_0.99.12.tar.gz macOS 12.7.1 Monterey: DeepTarget_0.99.12.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 2a9d4d0970c0b599bb89db961aacb8daa4af1f56

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.2 LTS): DeepTarget_0.99.13.tar.gz macOS 12.7.1 Monterey: DeepTarget_0.99.13.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[TrinhNguyenP]

Hi @TrinhNguyenP,

Thank you for submitting DeepTarget to Bioconductor. I'm afraid that in its current state it is not suitable for Bioconductor and would require substantial changes before it could be accepted.

In my review below I have separated the issues into Required and Recommended points that I would ask you to address before the package can be accepted. 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.

Cheers, Pete

Required

• [ ] From where are the datasets originally available and documented/explained? The README says, "For full datasets used in the manuscript, please contact us at tinh.nguyen@nih.gov. For the detail of the links for downloading the data, please do ??DeepTarget." Why can't this information be included in the package instead of users having to email someone to get the full datasets? Furthermore, running ??DeepTarget doesn't tell a user anything useful; running ?OntargetM has some poorly formatted plain text links to files on the www.depmap.org but with no real context or explanation.
• [ ] Where's the full dataset available in this package? The DESCRIPTION says "This package spans about 1500 drugs across about 18K possible target genes" but the included data (OntargetM) is seemingly much smaller.

> str(OntargetM)
List of 5
 $ DrugMetadata   :'data.frame':  11 obs. of  5 variables:
  ..$ broad_id_trimmed: chr [1:11] "A27883417" "K02113016" "K09951645" "K38527262" ...
  ..$ name            : Factor w/ 4518 levels "1-((Z)-3-Chloroallyl)-1,3,5,7-tetraazaadamantan-1-ium",..: 250 2977 1213 462 3081 3537 3583 313 2065 3311 ...
  ..$ target          : Factor w/ 1830 levels "AADACL2","AADAT, AASS, ABAT, ALDH18A1, ASNS, BCAT1, BCAT2, CCBL2, DNPEP, EARS2, ENPEP, EPRS, FOLH1, FPGS, FTCD, GAD1, GAD"| __truncated__,..: 1648 1525 460 1308 604 1168 1399 1418 453 1174 ...
  ..$ drug_category   : Factor w/ 3 levels "chemo","noncancer",..: 3 3 3 3 3 3 3 3 3 2 ...
  ..$ moa             : Factor w/ 1072 levels "11-beta hydroxysteroid dehydrogenase inhibitor",..: 834 825 905 644 259 563 700 697 217 590 ...
 $ secondary_prism: num [1:16, 1:392] 0.864 1.335 NA 1.518 0.955 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:16] "A27883417" "K02113016" "K02113016" "K09951645" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...
 $ avana_CRISPR   : num [1:487, 1:392] 0.1191 0.0405 -0.1317 0.0816 -0.1112 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:487] "ABL2" "ACAT2" "ACCSL" "ACTA1" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...
 $ mutations_mat  : num [1:476, 1:392] 0 0 0 0 0 0 0 0 0 0 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:476] "ABL2" "ACAT2" "ACCSL" "ACTA1" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...
 $ expression_20Q4: num [1:550, 1:392] 6.13 2.12 7.12 1.35 6.46 ...
  ..- attr(*, "dimnames")=List of 2
  .. ..$ : chr [1:550] "CD99" "TMEM176A" "POLDIP2" "USH1C" ...
  .. ..$ : chr [1:392] "ACH-000461" "ACH-000528" "ACH-000792" "ACH-000570" ...

• [ ] The vignette is inadequate. Some of the issues include:

  • There's not enough explanation of what the code examples are doing or why.
  • Outputs (e.g., plots and previews of data.frame objects) lack sufficient explanation or guidance on interpretation.
  • The vignette should include a detailed introduction/abstract section that provides motivation for inclusion in Bioconductor and when appropriate a review and comparison to existing Bioconductor packages with similar functionality or scope.
  • Random commented out lines, e.g., dir.create(), saveRDS().
• [ ] Please proofread the documentation and vignette, as well as run a spell checker on the package (if you use RStudio then there is one built in to help). Imagine a new user trying to use your package and make it as simple for them as possible by not distracting them with strange formatting and typos.
• [ ] For inclusion in Bioconductor, parallelisation should be with BiocParallel (e.g., use BiocParallel::bplapply() rather than parallel::mclapply() in DoPWY(); there are other uses of mclapply() that should also be changed); see https://contributions.bioconductor.org/r-code.html#parallel-recommendations. Furthermore, functions must not hard-code the parallelisation option (e.g., DoPWY() hardcodes mc.cores = 2 when computing DoPWYEnr).
• [ ] What is inst/ExWorkflow.R? Please remove it from the git repo or explain why it is required.
• [ ] Please use a more descriptive filename for the vignette than V_analyses.Rmd.

• [ ] The man pages are inadequate. Some of the issues include but are not limited to:

  • Function arguments are not adequately documented. E.g., ?DoPWY says that Sim.GES.DRS is the result of CorCRISPRDrugR(), a function that doesn't exist in DeepTarget.
  • GetSim() says it returns a data.frame but returns a list of matrix objects.
  • PredTarget() says it returns a table but returns a data.frame and it is not documented what the column names mean.
  • The examples for ?colorBar and ?line2user are very long and yet still inadequate because they don't clearly exemplify how/why to use these fucntions.

• [ ] Function names are uninformative.

  • E.g., According to the man page, GetSim() "Compute[s] a correlation between the every gene crispr KO vs each drug response". Leaving aside the explanation, which I don't understand, I can't see how 'GetSim' is a good name for this function.
  • Unexplained acronyms and abbreviations used throughout, e.g., DMB(), DTR() etc.

• [ ] The cor.test_trimmed_v0.default() function is a big red-flag that this package requires substantial revision before it would be suitable for Bioconductor.

  • It seems to have copy-pasted code from stats::cor.test(). Is this correct? Why is this function needed. You can't just copy+paste code without attribution (not to mention it being subject to the original code's licence).
  • Furthermore, it seems like cor.test_trimmed_v0.default() shouldn't be exported because it's documented as "This will be called inside other function such as Getsim".

• [ ] The errHandle() function is another red-flag.

  • It's documented as err_handle() and the documentation is uninformative.
  • It's basically tryCatch().
  • Why is this exported?

• [ ] The fdrCor() function is another red-flag.

  • The documentation is inadequate and doesn't credit p.adjust().
  • It's basically p.adjust().
  • Why is this exported?
• [ ] BiocCheck::BiocCheck() generates 15 NOTES, many of which are easily addressed and so should be.
• [ ] The README is poorly formatted and lacking structure. Please fix.
• [ ] The DESCRIPTION contains typos, incorrect information, and is hard to understand.
• [ ] In NAMESPACE, please use selective imports using importFrom instead of import all with import.

• [ ] Exported data and the data/ directory must be well documented.

  • OntargetM documentation is inadequate.
  • Says, "A list of 5 matrices" but it's actually a list of 1 data.frame and 4 matrix objects.
  • There's no explanation of column names.
  • Poor formatting makes it hard to read documention.
  • Documentation doesn't match data. The dimensions of objects don't match. E.g., nrow(OntargetM$avana_CRISPR) is 487 but documented as 488, nrow(OntargetM$mutations_mat) is 476 but documented as 470

• [ ] There's no obvious interoperability with other Bioconductor packages.

  • The data structures used in the package are ad hoc; are there existing Bioconductor data structures you could re-use?
  • Could you show/explain in the vignette how this package might be used in conjunction with other Bioconductor workflows and software

Recommended

• [ ] It is strongly recommended to add unit tests; see https://contributions.bioconductor.org/tests.html.
• [ ] Some functions return data.table objects (e.g., DoPWY()) and others a data.frame. It would be better to be consistent. This should be precisely documented in the 'Value' section of each man page.
• [ ] The example from plotSim() is poorly/weirdly formatted.
• [ ] There's no need to use print() of code comments in the vignette (e.g., print ( " we will focus on these drugs:");); a plain comment (e.g., # For the vignette we will focus on the following drugs) is sufficient.
• [ ] The code is quite hard to read and would benefit from following a consistent coding style. See https://contributions.bioconductor.org/r-code.html#coding-style for the Bioconductor code style guide, although this it not the only acceptable style. But code like S.Drug <- OntargetM$DrugMetadata$broad_id_trimmed [idx.Drug] is quite hard to read and makes its harder for a user to follow.
• [ ] Recommend omitting LazyData: true in DESCRIPTION; see https://contributions.bioconductor.org/description.html#description-lazydata.
• [ ] Consider adding a inst/CITATION; see https://contributions.bioconductor.org/citation.html.

Hi @PeteHaitch, We have revised according to your required and recommended items. Please find attached file. There is a reponse for each item. Please let me know if there is anything unclear or if there is any other suggestion for improvement. Best, Trinh Bio_ResponseTo_Reviewer.docx

[PeteHaitch]

Could you please post your response as text directly into the issue rather than sharing as .docx file.

[TrinhNguyenP]

Could you please post your response as text directly into the issue rather than sharing as .docx file.

Hi @PeteHaitch, I am sorry I missed this response. Here it is:

---

Required

• [x] From where are the datasets originally available and documented/explained? The README says, "For full datasets used in the manuscript, please contact us at tinh.nguyen@nih.gov. For the detail of the links for downloading the data, please do ??DeepTarget." Why can't this information be included in the package instead of users having to email someone to get the full datasets? Furthermore, running ??DeepTarget doesn't tell a user anything useful; running ?OntargetM has some poorly formatted plain text links to files on the www.depmap.org but with no real context or explanation.
• [x] Where's the full dataset available in this package? The DESCRIPTION says "This package spans about 1500 drugs across about 18K possible target genes" but the included data (OntargetM) is seemingly much smaller.

Explanation: We used 1500 drugs across about 18K possible target genes for the manuscript. Due to limited size Rdata embedded in the package, we used only part of data for demonstration in the submission. We have removed the number of drugs and genes to avoid confusion from the DESCRIPTION. Alternatively, users can download public datasets from depmap.org or their own data as an input to this package. Therefore, the number of drug or genes is not limited. We have made some changes in the DESCRIPTION as below: Description: This package predicts a drug’s primary target(s) or secondary target(s) by integrating large-scale genetic and drug screens from the Cancer Dependency Map project run by the Broad Institute. It further investigates whether the drug specifically targets the wild-type or mutated target forms. To show how to use this package in practice, we provided sample data along with step-by-step example. We also guided how to interpret the result. The full dataset is available in www.depmap.org and user can do ??OntargetM for the specific link for the data we used in the paper. To make it clearer, we have revised its Rmd. Please see the screenshot as below:

• [x] The vignette is inadequate. Some of the issues include:
  • There's not enough explanation of what the code examples are doing or why.
  • Outputs (e.g., plots and previews of data.frame objects) lack sufficient explanation or guidance on interpretation.
  • The vignette should include a detailed introduction/abstract section that provides motivation for inclusion in Bioconductor and when appropriate a review and comparison to existing Bioconductor packages with similar functionality or scope.

Response: We have included more detail in the introduction section.

• Random commented out lines, e.g., dir.create(), saveRDS().

Response: We have removed those.

• [x] Please proofread the documentation and vignette, as well as run a spell checker on the package (if you use RStudio then there is one built in to help). Imagine a new user trying to use your package and make it as simple for them as possible by not distracting them with strange formatting and typos.

Response: We used check spelling in R studio and have made all corrections where applicable for all *.Rmd and vignette. We also removed commented outlines. We also added more explanation throughout the vignette. Thanks for your suggestion.

• [x] For inclusion in Bioconductor, parallelisation should be with BiocParallel (e.g., use BiocParallel::bplapply() rather than parallel::mclapply() in DoPWY(); there are other uses of mclapply() that should also be changed); see https://contributions.bioconductor.org/r-code.html#parallel-recommendations. Furthermore, functions must not hard-code the parallelisation option (e.g., DoPWY() hardcodes mc.cores = 2 when computing DoPWYEnr).

Response: We have replaced all mclapply with BiocParallel::bplapply() and removed the mc.cores=2.

• [x] What is inst/ExWorkflow.R? Please remove it from the git repo or explain why it is required.

Response: We have removed it. Thanks.

• [x] Please use a more descriptive filename for the vignette than V_analyses.Rmd.

Response: We have revised it to “DeepTarget_Vignette.Rmd”

• [ ] The man pages are inadequate. Some of the issues include but are not limited to:
  • Function arguments are not adequately documented. E.g., ?DoPWY says that Sim.GES.DRS is the result of CorCRISPRDrugR(), a function that doesn't exist in DeepTarget.
  • GetSim() says it returns a data.frame but returns a list of matrix objects.
  • PredTarget() says it returns a table but returns a data.frame and it is not documented what the column names mean.
  • The examples for ?colorBar and ?line2user are very long and yet still inadequate because they don't clearly exemplify how/why to use these fucntions.

Response: We have decided to keep only the colorBar function. The colorBar function helps to add the color bar placed in the middle of the plot automatically. The size of color bar will be adjusted automatically depending on the size of the plot. Since we use this function in the plotSim function, we have removed its Rd from the man directory. We also removed exporting this function from NAMESPACE file. Thanks very much.

• [x] Function names are uninformative.
  • E.g., According to the man page, GetSim() "Compute[s] a correlation between the every gene crispr KO vs each drug response". Leaving aside the explanation, which I don't understand, I can't see how 'GetSim' is a good name for this function.

Response: We have changed to computeCor.

• Unexplained acronyms and abbreviations used throughout, e.g., DMB(), DTR() etc.

Response:

DMB: DMB stands for Drug Mutant Binding. This function will help to determine whether a drug is likely to bind to the mutant or non-mutant based on the correlation values and p values from lm model. This also generates the plot for visualization. Please see the screenshot of its Rmd as below:

DTR: DTR stands for Drug Target Response: This function will help to visualize whether this drug is likely response to a gene in cell lines where that gene is expressed and not in cell lines where that gene is not expressed. User can select a desired cut-off. Please see the screenshot of its Rmd as below:

• [x] The cor.test_trimmed_v0.default() function is a big red-flag that this package requires substantial revision before it would be suitable for Bioconductor.
  • It seems to have copy-pasted code from stats::cor.test(). Is this correct? Why is this function needed. You can't just copy+paste code without attribution (not to mention it being subject to the original code's licence).
  • Furthermore, it seems like cor.test_trimmed_v0.default() shouldn't be exported because it's documented as "This will be called inside other function such as Getsim".

Response: We have removed this from namespace as well.

• [x] The errHandle() function is another red-flag.
  • It's documented as err_handle() and the documentation is uninformative.
  • It's basically tryCatch().
  • Why is this exported?

Response: we have removed err_handle() and replace this with tryCatch(). Also, we have removed from namespace.

• [x] The fdrCor() function is another red-flag.
  • The documentation is inadequate and doesn't credit p.adjust().
  • It's basically p.adjust().
  • Why is this exported?

Response: we have removed fdrCor() and replace this with p.adjust(). Also, we have removed from namespace.

• [ ] BiocCheck::BiocCheck() generates 15 NOTES, many of which are easily addressed and so should be.

RESPONSE: We have fixed most of the NOTES, including a replacement of sapply to vapply, 1:nrow etc.. We also added a NEWS.md file. There is a note “Consider shorter lines” in the Rmd files, we didn’t fix it due to the clearer explanation to the users.

• [x] The README is poorly formatted and lacking structure. Please fix.

Response: We have fixed the README file.

• [x] The DESCRIPTION contains typos, incorrect information, and is hard to understand.

Response: We have checked all the typos and fixed if found. We also revised as we mentioned above.

• [x] In NAMESPACE, please use selective imports using importFrom instead of import all with import.

Response: We have removed all import all. Replace all w specific importFrom instead when it is relevant.

• [x] Exported data and the data/ directory must be well documented.
  • OntargetM documentation is inadequate.
  • Says, "A list of 5 matrices" but it's actually a list of 1 data.frame and 4 matrix objects.
  • There's no explanation of column names.
  • Poor formatting makes it hard to read documention.
  • Documentation doesn't match data. The dimensions of objects don't match. E.g., nrow(OntargetM$avana_CRISPR) is 487 but documented as 488, nrow(OntargetM$mutations_mat) is 476 but documented as 470

Response: We have revised so it would matched with the data provided in the package. Please see the screenshot as below:

• [x] There's no obvious interoperability with other Bioconductor packages.
  • The data structures used in the package are ad hoc; are there existing Bioconductor data structures you could re-use?

Response: We don’t recognize there is an existing Bioconductor data structures we could re-use for this package.

• Could you show/explain in the vignette how this package might be used in conjunction with other Bioconductor workflows and software

Response: These is one package named depmap has been released recently in bioconductor. Users can use this package to download data and then use this as input to this Deeptarget package. The link is shown below: https://bioconductor.org/packages/release/data/experiment/vignettes/depmap/inst/doc/depmap.html

Recommended

• [ ] It is strongly recommended to add unit tests; see https://contributions.bioconductor.org/tests.html.
• [ ] Some functions return data.table objects (e.g., DoPWY()) and others a data.frame. It would be better to be consistent. This should be precisely documented in the 'Value' section of each man page.

Response: DoPWY() function returns dataframe for each drug.

• [ ] The example from plotSim() is poorly/weirdly formatted.

Response: We have fixed the format.

• [ ] There's no need to use print() of code comments in the vignette (e.g., print ( " we will focus on these drugs:");); a plain comment (e.g., # For the vignette we will focus on the following drugs) is sufficient.

Response: We have removed print()

• [x] The code is quite hard to read and would benefit from following a consistent coding style. See https://contributions.bioconductor.org/r-code.html#coding-style for the Bioconductor code style guide, although this it not the only acceptable style. But code like S.Drug <- OntargetM$DrugMetadata$broad_id_trimmed [idx.Drug] is quite hard to read and makes its harder for a user to follow.

Response: We have improved the scripts and added more explanation throughout the vignette. Please see more detail in the vignette file.

• [x] Recommend omitting LazyData: true in DESCRIPTION; see https://contributions.bioconductor.org/description.html#description-lazydata.

Response: we have removed the line in the DESCRIPTION.

• [x] Consider adding a inst/CITATION; see https://contributions.bioconductor.org/citation.html.

Response: We have added inst/CITATION

[PeteHaitch]

Hi Trinh,

I need to let you know that this review is unlikely to happen until the new year. I have 5 days left of work for the year, and quite a few commitments due in those days, and I'll then be on leave until January 8.

The next Bioconductor release not for several months, so there will still be plenty of time to complete the review process by that time.

Cheers, Pete

[TrinhNguyenP]

Hi Trinh,

I need to let you know that this review is unlikely to happen until the new year. I have 5 days left of work for the year, and quite a few commitments due in those days, and I'll then be on leave until January 8.

The next Bioconductor release not for several months, so there will still be plenty of time to complete the review process by that time.

Cheers, Pete

Hi @PeteHaitch, Thanks for letting me know. Enjoy your holiday! Best, Trinh

[vjcitn]

The review of this package has been very thorough; thanks @PeteHaitch ! I believe there is more work to be done to make the package fit well within the Bioconductor ecosystem, so I will mark the package with "needs interop".

Of particular interest is the depmap package that can already provide data on the crispr screens in the Broad resource.

I'd ask @TrinhNguyenP to look closely at this and other Bioconductor packages related to pharmacogenomics and consider how to reduce introducing de novo processes and structures that are already available.

[TrinhNguyenP]

The review of this package has been very thorough; thanks @PeteHaitch ! I believe there is more work to be done to make the package fit well within the Bioconductor ecosystem, so I will mark the package with "needs interop".

Of particular interest is the depmap package that can already provide data on the crispr screens in the Broad resource.

I'd ask @TrinhNguyenP to look closely at this and other Bioconductor packages related to pharmacogenomics and consider how to reduce introducing de novo processes and structures that are already available.

Hi @vjcitn, Thanks for your suggestion. I will take a look at depmap and other Bioconductor packages and will get back to you. Trinh

[bioc-issue-bot]

Dear @TrinhNguyenP ,

We have reopened the issue to continue the review process. Please remember to push a version bump to git.bioconductor.org to trigger a new build.

[bioc-issue-bot]

A reviewer has been assigned to your package for an indepth review. Please respond accordingly to any further comments from the reviewer.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: cf89dcebf2ace3802185bb082925f3ac50736150

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

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.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): DeepTarget_0.99.14.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[TrinhNguyenP]

Hi @PeteHaitch, We added a function named Depmap2deeptarget to retrieve data from Depmap and then do the pre-processing if applicable. Right now, there are only 3 data types available with older version available on Depmap. They are: Crispr, mutation, and gene expression level. I have contacted the authors to ask for an update for these three files. I also asked the authors to make two additional files available, which are Viability after Drug Treatment and Drug metadata from Broad. I have requested 2 weeks ago. When I receive their update, I will update Depmap2deeptarget function. Thanks, Trinh

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 70ca8271de67041b1a4b7d1b46fcf833bdfe2171

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): DeepTarget_0.99.15.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[TrinhNguyenP]

Good morning @PeteHaitch,

Can you please give me an update on the submission? Please let me know if Additional explanation about integration of Depmap data as an input for Deeptarget package is needed.

Thanks, Trinh

[PeteHaitch]

@TrinhNguyenP I am aiming to resume the review in the next few days.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 4cb8f40f5826bd256739e289dfa79130d0f63e13

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

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 build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): DeepTarget_0.99.16.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: 0b53fff2da086c950246f29b9f69cda7279e8a88

[bioc-issue-bot]

Dear Package contributor,

This is the automated single package builder at bioconductor.org.

Your package has been built on the Bioconductor Single Package Builder.

Congratulations! The package built without errors or warnings on all platforms.

Please see the build report for more details.

The following are build products from R CMD build on the Single Package Builder: Linux (Ubuntu 22.04.3 LTS): DeepTarget_0.99.17.tar.gz

Links above 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/DeepTarget to trigger a new build. A quick tutorial for setting up remotes and pushing to upstream can be found here.

[TrinhNguyenP]

@TrinhNguyenP I am aiming to resume the review in the next few days.

Hi @PeteHaitch, The Author of Depmap has updated the package and suggested me to use the package by downloading any of the original files provided by the DepMap project on Figshare (https://figshare.com/authors/Broad_DepMap/5514062). I have implemented this into the function Depmap2DeepTarget. All the required files for DeepTarget are not available in one version. Therefore, the function allows users to download each data type for a desired version, following the data preparation for DeepTarget as input. Please find this update on this submission.

Please let me know if you have question.

Thanks,

Trinh

[PeteHaitch]

Hi @TrinhNguyenP,

I'm currently on leave but I'll do my best to get the review shortly.

Quick question: do you really need to import the entire tidyverse? Can you please try to minimise the dependencies by only using the packages from the tidyverse you actually need and then only importing the necessary functions from those packages.

Thanks, Pete

[TrinhNguyenP]

Hi @TrinhNguyenP,

I'm currently on leave but I'll do my best to get the review shortly.

Quick question: do you really need to import the entire tidyverse? Can you please try to minimise the dependencies by only using the packages from the tidyverse you actually need and then only importing the necessary functions from those packages.

Thanks, Pete

Hi @PeteHaitch,

I have trouble locating which pacakge from tidyverse. I am checking w Laurent and I will spend some more time working on it. I will add this on the list for the next version.

Thanks,

Trinh

[vjcitn]

Pete is on leave and Lori and I will continue the review.

Here, the script loads OntargetM object and prepare the drug response scores 
(secondary_prism ) matrix for the interesting drugs and Gene effect scores after 
CRISPR-KO matrix.

is in the vignette. This sentence doesn't really make sense (scores after ... matrix)? What is the meaning of "after" here? Where is the process of the creation of OntargetM documented? The online sources are given but what filtering was applied and are any upstream changes likely?

There is a very large data dump in vignette section 3.2. Please make this more concise.

The multiple uses of sapply should be replaced by vapply calls.

More comments on the vignette will be forthcoming. Thanks for this contribution.

[TrinhNguyenP]

Pete is on leave and Lori and I will continue the review.

Here, the script loads OntargetM object and prepare the drug response scores 
(secondary_prism ) matrix for the interesting drugs and Gene effect scores after 
CRISPR-KO matrix.

is in the vignette. This sentence doesn't really make sense (scores after ... matrix)? What is the meaning of "after" here? Sorry for confusion. it just simply a matrix of CRISPR Gene Effect scores.

Where is the process of the creation of OntargetM documented? The online sources are given but what filtering was applied and are any upstream changes likely? **_The process of the creation of OntargetM documented in OntargetM.Rd. This includes the information for filtering and any upstream conversion if needed. For example, for the item of mutation matrix: \item{\code{mutations_mat}}{Mutation binary matrix for 476 genes as row names across 392 unique cell lines as column names; 0 is WT; 1 is mutated} The original downloaded file contains the type of mutations. Users can do it by themselves. Alternately, User can use Depmap2Deeptarget function instead. This function also takes care of removing "SILENT" mutation. For example, users wants to use version 22Q4: CLLE.mut = Depmap2DeepTarget("OmicsSomaticMutations.csv","22Q4")._**

There is a very large data dump in vignette section 3.2. Please make this more concise. I will take a look.

The multiple uses of sapply should be replaced by vapply calls. I will work on it. More comments on the vignette will be forthcoming. Thanks for this contribution.

Hi @vjcitn, Please see my response in the italic bolded. Best, Trinh

[TrinhNguyenP]

Hi @vjcitn, I would love to check to see whether I am expecting more comments on the vignette. Thanks, Trinh

[vjcitn]

Notice the big data dump that persists in 0.99.17 in section 3.2. What is the reader to do with this? Please summarize it. Thank you.

[TrinhNguyenP]

Notice the big data dump that persists in 0.99.17 in section 3.2. What is the reader to do with this? Please summarize it. Thank you.

Are you refering the code: head( DRS)? If this is the case, Instead of presenting the first few drugs, I can just show the example of presenting the initial ten outcomes pertaining to the first drug DRS[1,1:10].

Thanks, Trinh

[vjcitn]

yes, I do not believe the output of head(DRS) is useful in the vignette

[TrinhNguyenP]

yes, I do not believe the output of head(DRS) is useful in the vignette

Sure, I will do.

I will push an updated version ASAP if there is no other additional comments.

Thank You! Trinh

[vjcitn]

go ahead

[bioc-issue-bot]

Received a valid push on git.bioconductor.org; starting a build for commit id: ad433985915018362b7074ecaec3bc6bafb84895