Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Plot single value group #53

Merged

Conversation

billdenney
Copy link

Ensure that plotting shows up for single-value groups (related to #51)

This should be applied on top of #52 (review that one first)

Check the added test to see this visually. Generally, I ensured that the x-values were slightly lower and higher than the original values so that lines and ribbons showed up.

@codecov-commenter
Copy link

codecov-commenter commented Sep 12, 2023

Codecov Report

Merging #53 (b19e163) into master (23bd54a) will increase coverage by 0.03%.
Report is 7 commits behind head on master.
The diff coverage is 72.72%.

❗ Current head b19e163 differs from pull request most recent head c91078c. Consider uploading reports for the commit c91078c to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
+ Coverage   83.10%   83.13%   +0.03%     
==========================================
  Files           4        4              
  Lines        1095     1097       +2     
==========================================
+ Hits          910      912       +2     
  Misses        185      185              
Files Coverage Δ
R/vpcstats.R 77.17% <72.72%> (+0.08%) ⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

R/plot.R Outdated Show resolved Hide resolved
Copy link
Collaborator

@certara-jcraig certara-jcraig left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes requested to replace dplyr usage with data.table: https://github.com/certara/tidyvpc/pull/53/files#r1326571356

R/plot.R Outdated
#' single-value groups
#' @noRd
expand_vpc_stats_single_value <- function(vpc, xvar, width = 0.0001) {
d_vpc_stats <- vpc$strat
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@certara-jcraig Ah, this line should be vpc$stats not vpc$strat

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes a lot more sense! Updated dplyr commands to data.table.

expand_vpc_stats_single_value <- function(vpc, xvar, width = 0.0001) {
d_vpc_stats <- vpc$stats
if (!is.null(vpc$strat)) {
d_vpc_stats[, n_xvar := length(unique(get(xvar))), by = names(vpc$strat)]
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note that with data.table, we can modify d_vpc_stats directly, adding or updating the n_xvar column, without needing to reassign it with d_vpc_stats <- ...

@certara-jcraig certara-jcraig merged commit 7d18e37 into certara:master Oct 4, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants