Repository navigation
Fix duplicated x scale and expose cap width in PK line plots - #323
Conversation
gg_pkc_lineplot() added two scale_x_continuous() layers, so ggplot2 dropped the first and reported "Scale for x is already present" on every call with a numeric time_var. The expand setting carried by the discarded scale never applied. A single scale is now added, keeping that expand. Adds x_breaks to gg_pkc_lineplot() for time ranges too wide to label every timepoint, and a width argument to gg_add_stats() exposed as errorbar_width on gg_pkc_lineplot() and gg_lineplot(). All defaults preserve current output.
Unit Tests Summary 1 files 338 suites 1m 50s ⏱️ Results for commit 2067888. ♻️ This comment has been updated with latest results. |
Unit Test Performance Difference
Additional test case details
Results for commit a9eacd7 ♻️ This comment has been updated with latest results. |
Code Coverage SummaryDiff against mainResults for commit: 2067888 Minimum allowed coverage is ♻️ This comment has been updated with latest results |
|
Hi @Melkiades,
"We encourage to supply time_var as a factor, since it supports correct decimals formatting in the summary table."
Thanks, |
Error bars in gg_pkc_lineplot() were not dodged like the line/point layers, causing overlap across groups. Also removes a cli_inform() message that incorrectly nudged users to supply time_var/x as a factor, which contradicts the intended numeric usage. Co-authored-by: Claude <noreply@anthropic.com>
Adds a new PK box plot function with selectable whisker method (percentile, tukey, minmax), outlier detection, and an optional mean marker, documented and tested to match gg_pkc_lineplot()'s conventions.
|
✅ All contributors have signed the CLA |
|
I have read the CLA Document and I hereby sign the CLA |
Reword "5th/95th" to "5th and 95th" to avoid a spelling false-positive, and qualify the bare .time symbol with .data$ in group_by() to resolve the "no visible binding for global variable" NOTE.
Spellcheck also flagged "th" in "5th and 95th"; use "5% and 95%" instead.
Escape % in gg_pkc_boxplot.Rd (unescaped % starts a comment in Rd syntax) and fix the scale_x_continuous() link topic in gg_pkc_lineplot.Rd, per the CI "manuals not up-to-date" diff.
pkgdown::build_site() errors when an exported topic is missing from _pkgdown.yml's reference index.
|
It looks super good to me!! Thanks Zhen |
…oxplot() - Add `percentiles` parameter (default c(0.05, 0.95)) to customize whisker percentiles when whisker = "percentile" - Validate percentiles: must be two-element numeric, ordered, in [0,1] - Warn when non-positive values are removed for log_y = TRUE - Change dodge width from 0.8 to 1 so boxes touch - Add example showing custom 10%/90% percentiles
gg_pkc_lineplot(): - Add dodge_width parameter (default 0.2) to control horizontal separation between groups, replacing the hardcoded 0.2 value - Prevents overlap when errorbar_width > dodge_width gg_pkc_boxplot(): - Warn when non-positive values are removed for log_y = TRUE - Change dodge width from 0.8 to 1 so boxes touch - Change errorbar cap width from 0.8 to 1 to match
wzfrank
left a comment
There was a problem hiding this comment.
All functions are good and well tested.
…bility - ARD now uses switch(whisker) to produce self-describing stat names: percentile → p05/p25/p50/p75/p95 minmax → min/p25/p50/p75/max tukey → p25/p50/p75/iqr/lower_whisker/upper_whisker - Add optional id_var parameter; when supplied, outlier ARD rows carry the subject ID as stat_label (matching the template USUBJID convention)
…ale_and_caps@main
Replace generic ymin/ymax with descriptive bound names matching the template: mean_plus_sd/mean_minus_sd (sd), mean_plus_se/mean_minus_se (se), upper_ci/lower_ci (ci), q75/q25 (iqr). Align N capitalisation with the template. Minor comment cleanup in gg_pkc_boxplot.
What changes are proposed in this pull request?
gg_pkc_lineplot()now adds a single x scale instead of two, so it no longer reports "Scale for x is already present" on every call with a numerictime_var. Theexpandsetting that the discarded scale carried is now applied. (gg_pkc_lineplot()adds two x scales and hardcodes the error bar cap width #322)gg_pkc_lineplot()gains anx_breaksargument to set the breaks of a numeric x-axis, for time ranges too wide to label every timepoint. The defaultNULLkeeps a break on every observed timepoint. (gg_pkc_lineplot()adds two x scales and hardcodes the error bar cap width #322)gg_pkc_lineplot()andgg_lineplot()gain anerrorbar_widthargument to set the width of the error bar caps, previously fixed at0.45. (gg_pkc_lineplot()adds two x scales and hardcodes the error bar cap width #322)All three are backwards compatible: every default reproduces the current output.
Detail
The x scale bug was visible in our own test suite. Before this change,
Scale for x is already present. Adding another scale for x, which will replace the existing scale.appeared on essentially everygg_pkc_lineplot()test; after it, the full suite reports it zero times. Because ggplot2 kept the second scale, theexpand = expansion(mult = 0.05)on the first one had never had any effect.The two new arguments come from template work on PKCG03, where the lack of them was the stated reason for dropping
gg_pkc_lineplot()and hand-writing the plot in rawggplot2. That trade loses the input validation, the invalidstat/variabilityguard, the factor/numerictime_varhandling and the LLOQ line. Both gaps are small enough to close directly.x_breaksaccepts a vector or a break function, so a 0 to 600 hour profile can useseq(0, 600, by = 100)orscales::breaks_pretty()instead of a tick per timepoint.The three
man/*.Rdfiles were updated by hand: the local roxygen2 is 7.3.3 whileDESCRIPTIONpins 8.1.0, and running the older version rewrote unrelated files. If CI regenerates docs with 8.1.0 and shows a diff in those three files, that diff is formatting only.Reference GitHub issue associated with pull request. closes #322
Pre-review Checklist (if item does not apply, mark is as complete)
usethis::pr_merge_main()devtools::test_coverage()