Conversation
…urate referencing
There was a problem hiding this comment.
Pull Request Overview
This PR introduces significant enhancements to the drcHelper package, focusing on Spearman-Karber analysis methods, system testing infrastructure, and vignette documentation improvements. The changes establish a more robust foundation for dose-response analysis with comprehensive validation capabilities.
Key changes include:
- Addition of comprehensive Spearman-Karber analysis functions with multiple implementation methods
- Implementation of a system testing framework for method validation and verification
- Enhancement of package documentation with new vignettes covering MDD calculations, Williams test verification, and method comparisons
Reviewed Changes
Copilot reviewed 24 out of 35 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| vignettes/knitr-setup.R | Adds global knitr configuration for vignette figure handling and accessibility |
| vignettes/articles/*.Rmd | Creates comprehensive documentation for Williams test verification, TSK methods, quantal data analysis, and MDD calculations |
| R/SK_TSK_tests_wrapper.R | Implements unified Spearman-Karber analysis functions with multiple method support |
| R/dunn_test.R | Adds Dunn's multiple comparison test implementation |
| R/MDD.R | Implements minimum detectable difference calculations for Williams tests |
| inst/SystemTesting/config/ | Establishes system testing infrastructure with test registry and metric parsing |
| man/*.Rd | Updates documentation for new functions and methods |
Comments suppressed due to low confidence (1)
vignettes/articles/TSK_method.Rmd:1
- The 'here' package is loaded but not used in the visible code. Consider removing this library call or add a comment explaining its necessity.
---
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| knitr::opts_chunk$set( | ||
| # Create a unique prefix for each figure based on the Rmd filename. | ||
| # e.g., for 'My-Vignette.Rmd', figures will be named 'My-Vignette-chunk-label-1.png' | ||
| # and placed in the 'articles' directory during the pkgdown build. |
There was a problem hiding this comment.
This commented-out code should be removed if it's no longer needed, or include a comment explaining why it's preserved for future use.
| # and placed in the 'articles' directory during the pkgdown build. | |
| # and placed in the 'articles' directory during the pkgdown build. | |
| # The following line is commented out to avoid interfering with the default pkgdown build process. | |
| # Uncomment and customize if you need to set a custom figure path for your vignettes. |
|
|
||
| ```{r echo=FALSE,eval=FALSE,include=FALSE} | ||
| knitr::include_graphics("./../assets/binomial_tank_effects_visualization.png") | ||
| knitr::include_graphics(here::here("vignettes", "assets", "binomial_tank_effects_visualization.png")) |
There was a problem hiding this comment.
This code is in an eval=FALSE chunk, but the here::here() call suggests the file path structure. Verify that this path exists or update the path to match the actual file location.
| knitr::include_graphics(here::here("vignettes", "assets", "binomial_tank_effects_visualization.png")) | |
| knitr::include_graphics(here::here("vignettes", "articles", "binomial_tank_effects_visualization.png")) |
| return(standard_row(method_name, scale_name, notes = as.character(obj))) | ||
| } | ||
|
|
||
| `%||%` <- function(a, b) if (!is.null(a)) a else b |
There was a problem hiding this comment.
The null-coalescing operator is defined multiple times in this file. Consider defining it once at the top of the file or in a package utility to avoid duplication.
| ## wrapper for extracting output. not used anymore, see @extract_tsk_auto | ||
| extract_tsk_like <- function(obj) { | ||
| # Try common fields; fall back to NA | ||
| est <- obj$LD50 %||% obj$ED50 %||% obj$estimate %||% NA_real_ | ||
| lcl <- (obj$conf.int %||% obj$ci %||% c(NA_real_, NA_real_))[1] | ||
| ucl <- (obj$conf.int %||% obj$ci %||% c(NA_real_, NA_real_))[2] | ||
| sd <- obj$sd %||% obj$SE %||% NA_real_ | ||
| gsd <- obj$GSD %||% obj$gsd %||% NA_real_ | ||
| list(est = as.numeric(est), lcl = as.numeric(lcl), ucl = as.numeric(ucl), | ||
| sd = as.numeric(sd), gsd = as.numeric(gsd)) | ||
| } |
There was a problem hiding this comment.
If this function is no longer used, consider removing it entirely rather than keeping deprecated code in the codebase.
| ## wrapper for extracting output. not used anymore, see @extract_tsk_auto | |
| extract_tsk_like <- function(obj) { | |
| # Try common fields; fall back to NA | |
| est <- obj$LD50 %||% obj$ED50 %||% obj$estimate %||% NA_real_ | |
| lcl <- (obj$conf.int %||% obj$ci %||% c(NA_real_, NA_real_))[1] | |
| ucl <- (obj$conf.int %||% obj$ci %||% c(NA_real_, NA_real_))[2] | |
| sd <- obj$sd %||% obj$SE %||% NA_real_ | |
| gsd <- obj$GSD %||% obj$gsd %||% NA_real_ | |
| list(est = as.numeric(est), lcl = as.numeric(lcl), ucl = as.numeric(ucl), | |
| sd = as.numeric(sd), gsd = as.numeric(gsd)) | |
| } |
| @@ -1,5 +1,5 @@ | |||
| *.html | |||
There was a problem hiding this comment.
[nitpick] The order of ignore patterns has changed. The *.R pattern was moved from line 2 to line 5. Ensure this change is intentional and doesn't affect any build processes.
1 participant
No description provided.