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

fix two-sided p-value shading #437

Merged
merged 6 commits into from
Dec 31, 2021
Merged

fix two-sided p-value shading #437

merged 6 commits into from
Dec 31, 2021

Conversation

simonpcouch
Copy link
Collaborator

Fixes #424.

The geom_area height of p-value shading was twice the height of the respective histogram bars when the calculated statistic fell exactly on the boundaries of the right-most boundary of the left shaded region and left-most boundary of the right shaded region.🤯

This PR correct the p-value shading in those cases:

library(infer)

r_hat <- gss %>% 
  observe(college ~ sex, success = "no degree",
          stat = "ratio of props", order = c("female", "male"))

set.seed(33)

null_dist <- gss %>%
  specify(college ~ sex, success = "no degree") %>%
  hypothesize(null = "independence") %>% 
  generate(reps = 1000, type = "permute") %>% 
  calculate(stat = "ratio of props", order = c("female", "male"))


visualize(null_dist) +
  shade_p_value(obs_stat = r_hat, direction = "two-sided")

Created on 2021-12-30 by the reprex package (v2.0.0)

The fix nudges the right-hand boundary of the right-most area geom in the left-hand shading region 1/100000-th of the plot scale to the left. This should be visually indistinguishable, but will cause all of the (already failing) vdiffr tests to newly fail. Will come back to this once #428 is fixed.

Copy link
Collaborator

@mine-cetinkaya-rundel mine-cetinkaya-rundel left a comment

Choose a reason for hiding this comment

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

I like the nudging solution, seems simple enough and accomplishes the task.

@simonpcouch simonpcouch merged commit 62d9bb3 into main Dec 31, 2021
@simonpcouch simonpcouch deleted the viz-fix-424 branch December 31, 2021 19:43
@andrewpbray
Copy link
Collaborator

Phew, good find and a practical solution.

@github-actions
Copy link

This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue.

@github-actions github-actions bot locked and limited conversation to collaborators Jan 15, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug with visualize()
3 participants