search

ropensci / software-review

rOpenSci Software Peer Review.
287 stars 104 forks source link

Submission tidyqpcr - Quantitative PCR analysis in the tidyverse #470

Closed ewallace closed 2 years ago

ewallace commented 2 years ago

Date accepted: 2022-06-10 Submitting Author Name: Edward Wallace Submitting Author Github Handle: !--author1-->@ewallace<!--end-author1-- Other Package Authors Github handles: !--author-others-->@DimmestP<!--end-author-others-- Repository: https://github.com/ewallace/tidyqpcr Version submitted: 0.3.0 Submission type: Standard Editor: !--editor-->@jooolia<!--end-editor-- Reviewers: !--reviewers-list-->@kelshmo<!--end-reviewers-list--

Due date for @kelshmo: 2021-11-19 Archive: TBD Version accepted: TBD --- - Paste the full DESCRIPTION file inside a code block below: ``` Package: tidyqpcr Type: Package Title: Quantitative PCR Analysis with the Tidyverse Version: 0.3 Authors@R: c(person("Edward", "Wallace", email = "Edward.Wallace@ed.ac.uk", role = c("aut", "cre")), person("Sam", "Haynes", email = "samuel.haynes10@gmail.com", role = c("ctb"))) Description: This package is for reproducible quantitative PCR (qPCR) analysis using packages from the tidyverse, notably dplyr and ggplot2. It normalizes (by ddCq), summarizes, and plots pre-calculated Cq data, and plots raw amplification and melt curves from Roche LightCycler machines. It does NOT (yet) calculate Cq data from amplification curves. Depends: R (>= 3.1.0), tibble, tidyr Imports: rlang, dplyr, ggplot2, readr, forcats, assertthat, lifecycle Suggests: knitr, rmarkdown, tidyverse, cowplot, testthat VignetteBuilder: knitr License: Apache License 2.0 + file LICENSE LazyData: true RoxygenNote: 7.1.1 Encoding: UTF-8 URL: https://github.com/ewallace/tidyqpcr, https://ewallace.github.io/tidyqpcr/ BugReports: https://github.com/ewallace/tidyqpcr/issues Language: en-GB ``` ## Scope - Please indicate which category or categories from our [package fit policies](https://ropensci.github.io/dev_guide/policies.html#package-categories) this package falls under: (Please check an appropriate box below. If you are unsure, we suggest you make a pre-submission inquiry.): - [ ] data retrieval - [ ] data extraction - [ ] data munging - [ ] data deposition - [ ] workflow automation - [ ] version control - [ ] citation management and bibliometrics - [ ] scientific software wrappers - [x] field and lab reproducibility tools - [ ] database software bindings - [ ] geospatial data - [ ] text analysis - Explain how and why the package falls under these categories (briefly, 1-2 sentences): tidyqpcr is an R package that empowers scientists to conduct reproducible, flexible, and best-practice compliant quantitative polymerase chain reaction (qPCR) analysis. tidyqpcr offers a standardised user interface and structure for qPCR analysis, within the tidyverse paradigm of spreadsheet-like rectangular data frames and generic functions that build up complex analyses in a series of simple steps. - Who is the target audience and what are scientific applications of this package? Any molecular biologist or bioinformatician who needs to design or analyse a qPCR experiment. Quantitative PCR is among the most common techniques in biological and biomedical re- search, used for the quantification of DNA and RNA. Standardised and open-source qPCR analysis pipelines will encourage best-practices in the reporting of qPCR results, improve the evaluation of qPCR experiments and ultimately lead to increased confidence in conclusions based on qPCR data. - Are there other R packages that accomplish the same thing? If so, how does yours differ or meet [our criteria for best-in-category](https://ropensci.github.io/dev_guide/policies.html#overlap)? Some open-source libraries for qPCR analysis are available, notably [qpcR](https://github.com/anspiess/qpcR) (Spiess, 2018) and [pcr](https://github.com/MahShaaban/pcr) (Ahmed & Kim, 2018). qpcR is a feature rich qPCR analysis package relying on an object-oriented approach using S4 classes. pcr is a less extensive qPCR analysis package based on the tidyverse suite of generic data-science tools using the paradigm of tidy data (spreadsheet-like rectangular data frames). However, available packages either assume extensive prior R knowledge, overlook best-practices in qPCR experiments, or lack extensive documentation. There remains a need for a qPCR analysis package that integrates with the user-friendly tidyverse, encourages the use of MIQE best-practice compliant experimental design, and provides detailed example analysis pipelines as R vignettes. - (If applicable) Does your package comply with our [guidance around _Ethics, Data Privacy and Human Subjects Research_](https://devguide.ropensci.org/policies.html#ethics-data-privacy-and-human-subjects-research)? Not applicable. - If you made a pre-submission inquiry, please paste the link to the corresponding issue, forum post, or other discussion, or @tag the editor you contacted. We presented tidyqpcr on the rOpenSci discussion page which [Stefanie Butland and Sean Hughes](https://discuss.ropensci.org/t/tidyqpcr-for-quantitative-pcr-analysis-pre-presubmission-community-enquiry/2056) kindly responded to. In response to @seaaan ’s comments we improved the vignettes, stuck to a consistent function naming convention and added functionality to calculate ddCq. We intend to add further functionality in future versions including: support for absolute quantification, support for multiple targets per well, and enabling the use of the plater package. In email correspondence with Stefanie, we believe Julia Gustavsen would be a perfect editor for our project as they reviewed Sean’s [plater](https://github.com/ropensci/software-review/issues/60) package. As for reviewers, we think someone with experience in conducting assays for RNA/DNA quantification and normalisation would be of benefit because of the emphasis on experimental design best-practices. ## Technical checks Confirm each of the following by checking the box. - [x] I have read the [guide for authors](https://devguide.ropensci.org/guide-for-authors.html) and [rOpenSci packaging guide](https://devguide.ropensci.org/building.html). This package: - [x] does not violate the Terms of Service of any service it interacts with. - [x] has a CRAN and OSI accepted license. - [x] contains a [README with instructions for installing the development version](https://ropensci.github.io/dev_guide/building.html#readme). - [x] includes [documentation with examples for all functions, created with roxygen2](https://ropensci.github.io/dev_guide/building.html#documentation). - [x] contains a vignette with examples of its essential functions and uses. - [x] has a [test suite](https://ropensci.github.io/dev_guide/building.html#testing). - [x] has [continuous integration](https://ropensci.github.io/dev_guide/ci.html), including reporting of test coverage using services such as Travis CI, Coveralls and/or CodeCov. ## Publication options - [x] Do you intend for this package to go on CRAN? - [ ] Do you intend for this package to go on Bioconductor? - [ ] Do you wish to submit an Applications Article about your package to [Methods in Ecology and Evolution](http://besjournals.onlinelibrary.wiley.com/hub/journal/10.1111/(ISSN)2041-210X/)? - [x] Do you wish to submit an Applications Article about your package to Journal of Open Source Software? We have included a paper.md file in the repository as per JOSS instructions to authors. ## Code of conduct - [x] I agree to abide by [rOpenSci's Code of Conduct](https://ropensci.github.io/dev_guide/policies.html#code-of-conduct) during the review process and in maintaining my package should it be accepted.
ropensci-review-bot commented 2 years ago

Thanks for submitting to rOpenSci, our editors and @ropensci-review-bot will reply soon. Type @ropensci-review-bot help for help.

ropensci-review-bot commented 2 years ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 2 years ago

Checks for tidyqpcr (v0.3)

git hash: 4bce0332

Important: All failing checks above must be addressed prior to proceeding

Package License: Apache License 2.0 + file LICENSE

1. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has: - code in R (100% in 6 files) and - 1 authors - 4 vignettes - no internal data file - 7 imported packages - 28 exported functions (median 8 lines of code) - 28 non-exported functions in R (median 11 lines of code) --- Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages The following terminology is used: - `loc` = "Lines of Code" - `fn` = "function" - `exp`/`not_exp` = exported / not exported The final measure (`fn_call_network_size`) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile. |measure | value| percentile|noteworthy | |:------------------------|-----:|----------:|:----------| |files_R | 6| 35.8| | |files_vignettes | 4| 94.3| | |files_tests | 5| 78.5| | |loc_R | 339| 32.0| | |loc_vignettes | 1014| 93.7| | |loc_tests | 368| 65.9| | |num_vignettes | 4| 96.0|TRUE | |n_fns_r | 56| 52.0| | |n_fns_r_exported | 28| 74.9| | |n_fns_r_not_exported | 28| 40.8| | |n_fns_per_file_r | 5| 60.4| | |num_params_per_fn | 3| 33.1| | |loc_per_fn_r | 10| 40.0| | |loc_per_fn_r_exp | 8| 19.7| | |loc_per_fn_r_not_exp | 11| 51.8| | |rel_whitespace_R | 10| 22.3| | |rel_whitespace_vignettes | 35| 97.8|TRUE | |rel_whitespace_tests | 23| 86.5| | |doclines_per_fn_exp | 36| 42.9| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 10| 24.0| | ---

1a. Network visualisation

Interactive network visualisation of calls between objects in package can be viewed by clicking here

2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

--- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) R CMD check generated the following notes: 1. checking for hidden files and directories ... NOTE Found the following hidden files and directories: .github These were most likely included in error. See section ‘Package structure’ in the ‘Writing R Extensions’ manual. 2. checking installed package size ... NOTE installed size is 12.7Mb sub-directories of 1Mb or more: doc 2.8Mb extdata 9.7Mb 3. checking DESCRIPTION meta-information ... NOTE License components with restrictions not permitted: Apache License 2.0 + file LICENSE R CMD check generated the following check_fails: 1. no_description_depends 2. rcmdcheck_not_permitted_license_restrictions 3. rcmdcheck_hidden_files_and_directories 4. rcmdcheck_reasonable_installed_size #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 79.72 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) No functions have cyclocomplexity >= 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 85 potential issues: message | number of times --- | --- Lines should not be more than 80 characters. | 85

Package Versions

|package |version | |:--------|:--------| |pkgstats |0.0.2.3 | |pkgcheck |0.0.2.53 |

Editor-in-Chief Instructions:

Processing may not proceed until the items marked with :heavy_multiplication_x: have been resolved.

noamross commented 2 years ago

@ropensci-review-bot assign @jooolia as editor

ropensci-review-bot commented 2 years ago

Assigned! @jooolia is now the editor

DimmestP commented 2 years ago

@ropensci-review-bot help

ropensci-review-bot commented 2 years ago

Hello @DimmestP, here are the things you can ask me to do:


# List all available commands
@ropensci-review-bot help

# Show our Code of Conduct
@ropensci-review-bot code of conduct
DimmestP commented 2 years ago

Helpful! @noamross or @jooolia

Hello!! Thank you for spending your time helping us improve tidyqpcr.

I have added a few commits to the main tidyqpcr repo that I think solve the remaining issues. How can I get the review bot to check again?

What is the process that it uses to check for function examples? I ran covr::package_coverage(type = "examples") locally and got 83.96% coverage. The 'missing' examples are from functions that are variations of other functions (which are documented with examples) and are linked together with a @describeIn tag. See scale_x_log2nice and scale_x_log10nice in the plot_helpers.R file.

jooolia commented 2 years ago

@ropensci-review-bot check package

ropensci-review-bot commented 2 years ago

Thanks, about to send the query.

ropensci-review-bot commented 2 years ago

:rocket:

Editor check started

:wave:

ropensci-review-bot commented 2 years ago

Checks for tidyqpcr (v0.3)

git hash: 737cca43

Package License: Apache License 2.0 + file LICENSE

1. Statistical Properties

This package features some noteworthy statistical properties which may need to be clarified by a handling editor prior to progressing.

Details of statistical properties (click to open)

The package has: - code in R (100% in 6 files) and - 1 authors - 4 vignettes - no internal data file - 7 imported packages - 28 exported functions (median 8 lines of code) - 28 non-exported functions in R (median 11 lines of code) --- Statistical properties of package structure as distributional percentiles in relation to all current CRAN packages The following terminology is used: - `loc` = "Lines of Code" - `fn` = "function" - `exp`/`not_exp` = exported / not exported The final measure (`fn_call_network_size`) is the total number of calls between functions (in R), or more abstract relationships between code objects in other languages. Values are flagged as "noteworthy" when they lie in the upper or lower 5th percentile. |measure | value| percentile|noteworthy | |:-----------------------|-----:|----------:|:----------| |files_R | 6| 35.8| | |files_vignettes | 0| 0.0|TRUE | |files_tests | 5| 78.5| | |loc_R | 339| 32.0| | |loc_tests | 368| 65.9| | |num_vignettes | 4| 96.0|TRUE | |n_fns_r | 56| 52.0| | |n_fns_r_exported | 28| 74.9| | |n_fns_r_not_exported | 28| 40.8| | |n_fns_per_file_r | 5| 60.4| | |num_params_per_fn | 3| 33.1| | |loc_per_fn_r | 10| 40.0| | |loc_per_fn_r_exp | 8| 19.7| | |loc_per_fn_r_not_exp | 11| 51.8| | |rel_whitespace_R | 9| 19.3| | |rel_whitespace_tests | 23| 86.5| | |doclines_per_fn_exp | 48| 60.4| | |doclines_per_fn_not_exp | 0| 0.0|TRUE | |fn_call_network_size | 10| 24.0| | ---

1a. Network visualisation

Interactive network visualisation of calls between objects in package can be viewed by clicking here

2. goodpractice and other checks

Details of goodpractice and other checks (click to open)

#### 3a. Continuous Integration Badges [![github](https://github.com/ewallace/tidyqpcr/workflows/R-CMD-check/badge.svg)](https://github.com/ewallace/tidyqpcr/actions) **GitHub Workflow Results** |name |conclusion |sha |date | |:-------------------------------|:----------|:------|:----------| |.github/workflows/draft-pdf.yml |success |737cca |2021-10-13 | |fair-software |success |737cca |2021-10-13 | |R-CMD-check |success |737cca |2021-10-13 | --- #### 3b. `goodpractice` results #### `R CMD check` with [rcmdcheck](https://r-lib.github.io/rcmdcheck/) R CMD check generated the following notes: 1. checking installed package size ... NOTE installed size is 12.7Mb sub-directories of 1Mb or more: doc 2.8Mb extdata 9.7Mb 2. checking DESCRIPTION meta-information ... NOTE License components with restrictions not permitted: Apache License 2.0 + file LICENSE 3. checking package subdirectories ... NOTE Found the following CITATION file in a non-standard place: CITATION.cff Most likely ‘inst/CITATION’ should be used instead. R CMD check generated the following check_fails: 1. no_description_depends 2. rcmdcheck_not_permitted_license_restrictions 3. rcmdcheck_citation_file_at_standard_place 4. rcmdcheck_reasonable_installed_size #### Test coverage with [covr](https://covr.r-lib.org/) Package coverage: 79.72 #### Cyclocomplexity with [cyclocomp](https://github.com/MangoTheCat/cyclocomp) No functions have cyclocomplexity >= 15 #### Static code analyses with [lintr](https://github.com/jimhester/lintr) [lintr](https://github.com/jimhester/lintr) found the following 94 potential issues: message | number of times --- | --- Lines should not be more than 80 characters. | 94

Package Versions

|package |version | |:--------|:--------| |pkgstats |0.0.2.16 | |pkgcheck |0.0.2.86 |

Editor-in-Chief Instructions:

This package is in top shape and may be passed on to a handling editor

jooolia commented 2 years ago

Hi @DimmestP, I re-ran the checks and all looks good. I will start looking for reviewers.

Regarding the examples, there is no pre-set % that we require (as far as I know and from re-reading the packaging guide), right now it is stated as "Include extensive examples in the documentation", so I will let the reviewers comment on whether they think some functions are missing examples or not.

Thanks! Julia

mpadge commented 2 years ago

@jooolia The above check is all :heavy_check_mark:, including

:heavy_check_mark: All functions have examples.

jooolia commented 2 years ago

ha you are right @mpadge! thanks for pointing that out.

jooolia commented 2 years ago

@ropensci-review-bot add @kelshmo to reviewers

ropensci-review-bot commented 2 years ago

@kelshmo added to the reviewers list. Review due date is 2021-11-19. Thanks @kelshmo for accepting to review! Please refer to our reviewer guide.

ropensci-review-bot commented 2 years ago

@kelshmo: If you haven't done so, please fill this form for us to update our reviewers records.

jooolia commented 2 years ago

@ropensci-review-bot add @wolski to reviewers

ropensci-review-bot commented 2 years ago

@wolski added to the reviewers list. Review due date is 2021-12-10. Thanks @wolski for accepting to review! Please refer to our reviewer guide.

ropensci-review-bot commented 2 years ago

@wolski: If you haven't done so, please fill this form for us to update our reviewers records.

jooolia commented 2 years ago

Thanks @wolski for agreeing to review this package.

kelshmo commented 2 years ago

Package Review

Please check off boxes as applicable, and elaborate in comments below. Your review is not limited to these topics, as described in the reviewer guide

Documentation

The package includes all the following forms of documentation:

Functionality

Estimated hours spent reviewing: 2

Review Comments

The name of the package is clear and I appreciate the thorough descriptive content in your README.

Notes on README

Vignettes

"Primers and probes calibration vignette" and "Multifactorial multi-plate qPCR analysis example"

No comments on the other vignettes!

Notes on contributing

Notes on function definitions

Notes on examples

Thank you Edward and Sam, nice work and great package! I'm happy to learn about codemeta.json too. I appreciate your patience with my review.

DimmestP commented 2 years ago

Thanks so much for reviewing our package @kelshmo, I am glad our hard work is paying off. I've had a quick look at your comments and they all seem very relevant (and relatively quick fixes). We will wait for the next review to come back before actioning any of them though.

wolski commented 2 years ago

@DimmestP Dear package developers. I fully agree with the comments made by @kelshmo. However, if they are a relatively quick fix, then please fix them quickly! First, we could avoid redundant comments from my side, and I could more easily focus on different aspects of your package.

For instance, I typically go through vignettes by executing code chunks, and relative paths don't work.

> plate_cq_extdata <- read_lightcycler_1colour_cq(
+   "../inst/extdata/Stuart_dAgr_glyS_spoVG_5S_individualWells_Cq.txt"
+   ) %>%
+   dplyr::filter(! well %in% wells_exclude ) %>% # exclude blank wells
+   dplyr::select(well, sample_info, cq) 
 Error: '../inst/extdata/Stuart_dAgr_glyS_spoVG_5S_individualWells_Cq.txt' does not exist in current working directory ('C:/Users/wolski/prog/tidyqpcr').
wolski commented 2 years ago

BDW., my understanding is that the review process should be interactive. So I will be getting back to you with comments and questions on this issue before submitting the "formal" review.

ewallace commented 2 years ago

@wolski thank you! @DimmestP and I will talk, and see if we can get these fixed quickly enough for your review timeline. We welcome your comments and questions.

jooolia commented 2 years ago

@ropensci-review-bot submit review https://github.com/ropensci/software-review/issues/470#issuecomment-974548665 time 2

ropensci-review-bot commented 2 years ago

Logged review for kelshmo (hours: 2)

jooolia commented 2 years ago

@kelshmo thank you very much for your review!

wolski commented 2 years ago

@ewallace @DimmestP

I already have a few observations about what you have written about tidyqpcr and other packages in the section:

"Are there other R packages that accomplish the same thing? If so, how does yours differ or meet our criteria for best-in-category?"

You explicitly mention the package qpcR and pcr.

Your write "qPCR analysis package relying on an object-oriented approach using S4 classes." I am not a big fan of APIs with "@" either. But are you sure you are talking about qpcR? I checked the CRAN page and observed no dependency on the "methods". I downloaded the tar.gz and searched the sources for the "setClass" and "@" string, and there are no hits. "qpcR" is not written in S4.

Furthermore, why do you say that "available packages either assume extensive prior R knowledge"? Does this indicate a GUI for tidyqpcr; a shiny application? Or can I accomplish the same task with tidyqpcr with fewer lines of code than when using the PCR package? Finally, the vignettes you include are not concise; on the contrary, you heavily use ggplot2 in the vignette, while ggplot2 does not have a flat learning curve.

I do not know if @seaaan means the same as I when he writing "Focus on functions provided by your package and what they do and how to use them" (e.g. in the multifactorial vignette, explain at greater length about normalizeqPCR, debaseline, getRdTall)", but I would say the same. https://discuss.ropensci.org/t/tidyqpcr-for-quantitative-pcr-analysis-pre-presubmission-community-enquiry/2056 This comment was made in May 2021 i.e., half a year ago, but I do not see any evidence of how you addressed them in the vignettes of tidyqpcr.

Also, I would like to understand better what do you mean "overlook best-practices in qPCR experiments"? Indeed, you refer to the "MIQE best-practice compliant experimental design". MIQE is a checklist of requirements to publish a qPCR experiment. There is Table 1 in the publication about MIQE you have referenced. Would you maybe tabulate which MIQE requirements "tidyqpcr" already covers using which functions, which would improve the package documentation? I am aware that this is a lot to ask, but could you please include the CRAN alternatives in this table to give substance to your claim that those alternatives of tidyqpcr overlook best practices. Most likely, you can reuse this table in an upcoming publication of tidyqpcr.

Finally, you say "or lack extensive documentation". Both of the packages you mention are, in my opinion, well documented, and both are published in peer-reviewed journals: https://peerj.com/articles/4473/ https://academic.oup.com/bioinformatics/article/24/13/1549/238435 I am not claiming that this guarantees their utility but documented they are.

Furthermore, a web search for R and qPCR points to several more R packages that you do not mention: https://bmcgenomics.biomedcentral.com/articles/10.1186/1471-2164-13-296 https://www.bioconductor.org/packages/release/bioc/html/HTqPCR.html

I feel that you did not address the points in https://devguide.ropensci.org/policies.html#overlap sufficiently, and I would welcome a critical comparison.

I will have a closer look at the three first points during the next week, but I have a few observations regarding: "Actively maintained while alternatives are poorly".

qPCR has a release history of 14 years; it has one reverse depends and two reverse imports, and four reverse suggests. On the other hand, the pcr package has a release history of 4 years and an active github repository.

Am I correct in assuming that tidyqpcr is a Ph.D. project of Samuel? Samuel is a gifted programmer, and for sure, you can publish this package in one of the many specialized journals. But how sustainable is this project? I am asking because tidyqpcr is by no means feature-complete (as the long list of To-Do's in README.md indicates). I find it worrying that there has been practically no activity within this project since 2019 https://github.com/ewallace/tidyqpcr/graphs/code-frequency

Furthermore, since their first release, all three other pinned projects on github.com/ewallace have not been significantly developed. Protocols were released in 2016 with about two commits/year since; stochsimode has seen no commits for six years. RoboViz has not had a single commit for three years.

jooolia commented 2 years ago

@kelshmo could you email me your current email address at j.gustavsen@gmail.com ? thanks!

jooolia commented 2 years ago

Thanks for the questions @wolski. I will let the authors reply. One thing that I want to note is that currently we do not require any discussion of the lifecycle of a package like we do for our statistical software review, but it is under discussion here: https://github.com/ropensci/dev_guide/issues/366

ewallace commented 2 years ago

@wolski Thank you for the thorough review.

You've made a lot of good points that our description of other packages was inaccurate, and we will address that at length. I created new issue ticket https://github.com/ewallace/tidyqpcr/issues/126, we will expand on that at some length and then get back to this thread.

Here I will answer quickly the points that can be answered quickly.

Your write "qPCR analysis package relying on an object-oriented approach using S4 classes." I am not a big fan of APIs with "@" either. But are you sure you are talking about qpcR? I checked the CRAN page and observed no dependency on the "methods". I downloaded the tar.gz and searched the sources for the "setClass" and "@" string, and there are no hits. "qpcR" is not written in S4.

That's our error that we will correct, qpcR uses S3 classes not S4 classes.

I do not know if @seaaan means the same as I when he writing "Focus on functions provided by your package and what they do and how to use them" (e.g. in the multifactorial vignette, explain at greater length about normalizeqPCR, debaseline, getRdTall)", but I would say the same. https://discuss.ropensci.org/t/tidyqpcr-for-quantitative-pcr-analysis-pre-presubmission-community-enquiry/2056 This comment was made in May 2021 i.e., half a year ago, but I do not see any evidence of how you addressed them in the vignettes of tidyqpcr.

That is fair, we have not expanded on these in the vignettes since May. What would "good enough" look like here for you? Are there particular points that need explaining from your perspective?

For context, the vignettes we have focus on the pain points that users reported in interviews; and on complete example data analyses. The big pain points were about laying out samples on plates and describing samples and genes accurately. Since we worked on those vignettes, users have reported it's much easier to use tidyqpcr, so that was a victory. But we have not written vignette-style documentation on the normalisation procedures and ct specifically.

We created a new issue to discuss adding to the vignettes: https://github.com/ewallace/tidyqpcr/issues/127

Am I correct in assuming that tidyqpcr is a Ph.D. project of Samuel? Samuel is a gifted programmer, and for sure, you can publish this package in one of the many specialized journals. But how sustainable is this project? I am asking because tidyqpcr is by no means feature-complete (as the long list of To-Do's in README.md indicates). I find it worrying that there has been practically no activity within this project since 2019 https://github.com/ewallace/tidyqpcr/graphs/code-frequency

tidyqpcr is a collaboration that I started and that has become part of Sam's PhD. We are planning to continue it after he graduates, and preparing for that by following best practices for software documentation and maintainability. Our next steps are:

The recent low activity on tidyqpcr is due to Sam focusing on another manuscript with analysis that makes extensive use of tidyqpcr, indeed substantiates the claims that tidyqpcr can take data all the way to publishable figures: https://github.com/DimmestP/chimera_project_manuscript

That manuscript is preprinted and will soon be submitted to a journal.

Furthermore, since their first release, all three other pinned projects on github.com/ewallace have not been significantly developed. Protocols were released in 2016 with about two commits/year since; stochsimode has seen no commits for six years. RoboViz has not had a single commit for three years.

You've identified a problem with my pinned projects, not so much with the software dev.

Ironically or otherwise, stochsimcode was my PhD code, which I put on GitHub because I was still getting occasional code requests from students doing masters projects.

riboviz has been under intensive development after moving to its own organisation riboviz/riboviz.

So a question for @wolski: what evidence would convince you of the future of this software? Is it a development plan with timeline? Or something else?

wolski commented 2 years ago

Best in category?

Some thoughts about the "best in category." Making a judgment about the best in the category would require extensive formal testing of all the relevant packages to avoid biases. Without it, the judgment would rely on heuristics only and be biased. Still, it is challenging to eliminate biases because of the complexity of package developments. As soon as I have more time, I will try to discuss, and hopefully, better understand the ropensci review process at https://discuss.ropensci.org/.

Here is my attempt to formalize the review.

Field and laboratory reproducibility tools

The package tidyqpcr was submitted in the category field and laboratory reproducibility tools.:

The family of "creation functions" of tidyqpcr falls into "Automation of field and lab protocols, such as sample tracking and tagging, form and datasheet generation," subcategory? My choice for this, however, would be PlateDesigner. https://www.ncbi.nlm.nih.gov/pmc/articles/PMC6821189/ for ease of use.

The functions: read_lightcycler_1colour_raw, ead_lightcycler_1colour_cq are in the subcategory: "interfacing with laboratory equipment or information systems, and executing experimental designs." The package ReadqPCR has more functionality in this subcategory.

But there is also a group of functions with names starting with scale_ and ending with nice, which relate to data visualization.

Finally, a group of functions starts with caculate_ which is statistics? Again, the CRAN package pcr implements a more comprehensive set and would be my preferred choice.

Package name

A comment regarding the package name tidy. tidy isn't just about using tidy data tables. It is also about grouping functions into tidy packages, one for plotting, a different one for factors, time, etc. tidyqpcr is an analysis script that reads analyses, and visualizes data, converted into a package. It feels pretentious to include the prefix tidy in the name of this package.

Past reviews

I hope there is no requirement to rewrite the suggestion made by @seaaan, more than 6 months ago, in my own words, to include it into this review process. Except for a few, those comments are still not addressed by the authors of tidyqpcr. I am not saying that you should implement all the recommendations, but I would appreciate it if you could explain why you decided not to.

I am listing them here:

Feature suggestions:

https://discuss.ropensci.org/t/tidyqpcr-for-quantitative-pcr-analysis-pre-presubmission-community-enquiry/2056

My comments

Vignettes

Rmarkdown and vignette title should be the same: You have: title: "Primers and probes calibration vignette" %\VignetteIndexEntry{PrimerCalibration} but better title: "Primers and probes calibration vignette" %\VignetteIndexEntry{Primers and probes calibration vignette}

The vignettes "Delta Cq 96-well plate qPCR analysis example" and "Introduction to designing an experiment and setting up a plate plan in tidyqpcr" is fine, good balance of code and text. However, there are almost no figure captions and text explaining the code in the vignettes: "Primers and probes calibration vignette," "Multifactorial multi-plate qPCR analysis example."

However, I have the impression that there are many redundancies in the vignettes, e.g., the display_plate function and the experiment setup are repeatedly covered. Simplifying the examples as @seaaan suggested will also help you meet the 5MB limit required by either CRAN or Bioconductor.

Connecting plate design and data reading.

The package contains many functions to create a plate design. They are then used to fill the plates using a multichannel pipette. The created annotations (e.g., WT, C+ ) can also be 'fed' into the measuring device. The sample_info column contains the annotation which was generated in the first step. Correct? 

##    well  sample_info      cq
##    <chr> <chr>         <dbl>
##  1 A1    WT OD0.5 glyS  14.0
##  2 A2    WT OD0.5 glyS  13.9
##  3 A3    WT OD0.5 glyS  14.0

Why can't you then not reproduce the plate design from the sample_info? Why is this code needed?

plate_cq_data <- 
  create_blank_plate_96well() %>% # row and column labels for each well
  inner_join(plate_cq_extdata) %>% # join that info to loaded data
  # next separate the sample and target information for future data analysis
  separate(sample_info,
           into = c("strain","OD","target_id"), 
           sep = " ",
           remove = FALSE) %>%
  unite("sample_id",strain, OD, remove = FALSE) %>% # put sample_id together
  mutate(prep_type = "+RT") # add information on cDNA prep type

I would like to have code like:

plate1 <- read_lightcycler_1colour_cq
display_plate(plate1)

display_plate

It always seems to color the wells according to the target_ID, and there is no option to choose a different column. Could you add an argument allowing to specify that column?

Confounding

If I am correct, thermocyclers are used to perform qPCR experiments. Aren't the wells in the plates' center exposed to different temperatures than on the edges? If so, how this impacts the quantification results? Does your design function takes this into account, and if not, why? Is there a theoretical option to block for the confounding of plate position?

Visualizing measurement results in the plate layout.

Would you please provide functions to visualize the results in plate layout, e.g., how those quantitation cycle, delta Cq looks on the plate? This would allow visualizing if there is a plate position effect.

Conclusion "Jack of all trades and a master of none."

I can not complete the review. I hypothesize that the reviewed package does duplicate large amounts of the functionality of other CRAN packages, which @ewallace must have known about in 2019, without making significant improvements. However, I do not have the time to prove or disprove it.

One possible direction for this package, would be to integrate several specialized packages, e.g. PlateDesigner, ReadqPCR, CRAN:pcr, using a common data structure (can be a tidy table). This integration would make it easier to perform MIQE compatible analysis. In addition, this package could implement functions to visualize the data easily. A possible implementation would start by annotating the tidy table, i.e., defining which columns contain plate positions, response variables, and explanatory variables. Then, using this information, figures (similar to those in the vignettes) could be created with a function call.

@jooolia, these two are experts in qPCR, which I am not. @seaaan, @MahShaaban

jooolia commented 2 years ago

Dear @wolski , thank you for the time that you have spent reviewing and the topics that you have raised for regarding this package and regarding the review process.

I would appreciate it if you could edit your latest comment so that it is more collegial. We are all here to help the authors improve their package by providing constructive comments. The sentences that you wrote in the Conclusion header and directly thereafter are not constructive.

Thank you for the experts in qpcr, however there would be a COI and thus they cannot be reviewers for this submission.

Finally, will you fill out the reviewer template (https://devguide.ropensci.org/reviewtemplate.html) or indicate that you are done with this round of your reviewing?

Thanks, Julia

wolski commented 2 years ago

@jooolia Thank you for your feedback, although I would have preferred to receive it by e-mail. But, yes, my comment "a script with a few helpers" without more context is not constructive. I was not referring to the package itself but the code equivalent. I am sorry.

I can not complete the review. I hypothesize that the reviewed package does duplicate large amounts of the functionality of other CRAN packages, which @ewallace must have known about in 2019, without making significant improvements. However, I do not have the time to prove or disprove it.

If open-source software does what I want, I will NOT start writing a new one. To me, there are those reasons to begin to write a new R package:

Let's assume we were in 2019 when the development of the reviewed package started. The packages qPCR, pcr, plateR or plateDesigner existed and must have been known to @ewallace. Specifically, the tidyqpcr::calculate_ functions are available elsewhere. Furthermore, their code isn't of low quality either, and it is debatable if tidyverse code, used in package implementations is easier to maintain.

Does the reviewed package provide a more flexible API or enable easier software components integration? I hypothesize that it is possible to rewrite the vignettes of the package under review, using other packages available in 2019, and that the code might be even 'tidier' (allowing for a few helper and wrapper functions). That's why I said: "analysis script with a few helper functions," which represents the code equivalent of the reviewed package. Unfortunately, however, I do not have the time to prove or disprove it.

ewallace commented 2 years ago

Thank you everyone.

After a call with @jooolia and discussion with @DimmestP, we have made a plan to address these reviews. We’ve raised a series of issue tickets on tidyqpcr repository, linked to this review ticket and also labeled rOpensci-review.

The plan is that in January we will start addressing those issues one by one. As we make progress there, we will request another round of feedback on this thread.

jooolia commented 2 years ago

Hi @wolski, thanks for your comment and for being clear that you will not continue to review here. I have removed you as a reviewer in the issue fields and feel free to unsubscribe from the issue if you do not want to continue to receive notifications about new comments. Thanks, Julia

jooolia commented 2 years ago

Thanks @ewallace the plan looks very clear to me regarding what you will tackle. Looking forward to the updates on your progress on the issues in 2022.

ewallace commented 2 years ago

Hello @jooolia and @kelshmo!

After a lot of work, @DimmestP and I are resubmitting our improved tidyqpcr package for review. We created a new tag tidyqpcr v0.4, for the latest version.

The issues we addressed are listed in the tidyqpcr repo under the rOpensci-review label.

We did not address new vignette on normalisation functions tidyqpcr#127, because we decided it was not essential and so prioritised other things.

We look forward to your feedback and review.

jooolia commented 2 years ago

Note: I contacted Kelsey Montgomery on LinkedIn to review the changes.

ewallace commented 2 years ago

Hello @jooolia is there any news about this review process?

jooolia commented 2 years ago

Hello @ewallace , I have been in contact with Kelsey who will have a look soon. I am having a look through the responses to the reviews and then will have a through the overall package. Apologies this has been going a bit more slowly than I expected (we rotate EiC duties and I am currently editor in chief at rOpenSci).

ewallace commented 2 years ago

Hello @jooolia, do let us know any news on this review, and if there's anything we can do to facilitate it?

kelshmo commented 2 years ago

Hi @ewallace - I will get this completed today. I apologize for the delay!

kelshmo commented 2 years ago

@ewallace Great work, thanks for your patience in working with me. Some communication early on fell through the cracks when I switched job roles!

Your addition of documentation and revised function names greatly helps with package clarity. I confirmed your use of file paths relative to the user function well. Your README and github pages look great. approved! Anything else you need from me?

ewallace commented 2 years ago

Thank you so much @kelshmo! I think the next steps are up to @jooolia.

jooolia commented 2 years ago

Thanks @kelshmo. Yes @ewallace I will move forward with this this week and will do a review. Thank you for your continued patience.

jooolia commented 2 years ago

Dear @ewallace, Thank you for your patience with my review.

This is part 1/2 of my review and will concern mostly aspects related to packaging. Part 2/2 will look more into the functions and the code (and more at the tests).

Check package integrity

run checks on tidyqpcr source:

devtools::check(pkg_dir)

This is mostly ok, but there are several files that should be added to your .Rbuildignore. ( ^codemeta.json$, ^CITATION.cff$, ^bibtex.bib$, ^paper.md$) so that you do not get notes when running the checks.

In your DESCRIPTION, the Apache license is one accepted by CRAN and rOpenSci, but it should written "Apache License (>= 2)" instead, you can use usethis::use_apache_license() to add the appropriate text to your DESCRIPTION and the file to your directory (and this will add LICENSE.md to .Rbuildignore). (Funnily enough it seems that Apache License (>=2) + file LICENSE is not ok, but MIT + file LICENSE doesn't raise any issues with check()).

run tests on tidyqpcr source:

devtools::test(pkg_dir)

Running tests using devtools::test() several tests fail (test-amp_melt_curve_functions.R:5:1,test-calculate_deltacq.R:5:1, test-plate_functions.R:6:1) because the needed packages are not loaded and not specified using ::, for example mutate and pull do not have references to dplyr.

Check package metadata files

Readme

The Readme is very comprehensive and provides a lot of information. For impatient people like me would you consider moving the Getting Started section closer to the top?

There is no code in the Readme so I think it is ok to have it as a .md.

Description

I see that on Github the version is 0.4 but in the description it is 0.3. Should this be updated? If you had different versions I would highly recommend a NEWS.md file to briefly describe the updates in the version (not requesting that you do this retrospectively, but would help with these changes).

Would consider moving tibble and tidyr to "imports" instead of "depends"? Or is there a specific reason for this?

Check documentation

test tidyqpcr function help files:

No top-level help-file exists. Please add one, more info here: https://devguide.ropensci.org/building.html#documentation

Review test suite:

test coverage

Currently no tests for two functions in read_qpcr_data.R and I think these are important functions for the package that need to be tested.

As I mentioned above this is a first part of the review. I will be away for Easter break, but will start working on part 2 upon my return.

Thanks, Julia