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

Change output of pariwise functions when nothing to compare, to allow for summarize use #62

Open
wfulp opened this issue Sep 23, 2020 · 4 comments
Milestone

Comments

@wfulp
Copy link
Contributor

wfulp commented Sep 23, 2020

Currently piarwise functions return NULL when only one group to compare. This breaks if using in group_by --> summarize. Could we change to data.frame of NAs instead?

[master] > control_testing_data %>%
+   group_by(day_plot, measurement, sample_type) %>%
+   summarise(pairwise_test_cont(x = magnitude, group = study_grouped),
+             .groups = 'drop') 
Error: Problem with `summarise()` input `..1`.
x Input `..1` must be a vector, not NULL.
i Input `..1` is `pairwise_test_cont(x = magnitude, group = study_grouped)`.
i The error occurred in group 1: day_plot = "Day 1", measurement = "sgmRNA", sample_type = "bal".
Run `rlang::last_error()` to see where the error occurred.

[master] > tmp_dat <- control_testing_data %>% 
+   filter(day_plot == "Day 1", measurement == "sgmRNA", sample_type == "bal")

[master] > pairwise_test_cont(x = tmp_dat $magnitude, group = tmp_dat $study_grouped)
NULL
@mayerbry
Copy link
Contributor

Can you provide a reprex for this issue?

I have two concerns:

  1. how experimental is this new stacking feature with summarize? What is it actually doing on the backend (i.e., how do you interpret "Input ..1 must be a vector, not NULL.")?
  2. we don't want to break old workflows, I'm guessing we return NULLs because of list stacking. Maybe not the best way to do things, but a different conversation :). The NA data.frame might be fine, but would be nice to experiment with a reprex.

@wfulp
Copy link
Contributor Author

wfulp commented Sep 23, 2020

Here is the wrapper I used as a workaround:


aa = function(...) {
  tmp_results = pairwise_test_cont(...)
  if (is.null(tmp_results)) {
    tmp_results = data.frame('Comparison' = NA, 
                             "SampleSizes" = NA,       
                             "Median_Min_Max" = NA,     
                             "Mean_SD" = NA,            
                             "MagnitudeTest" = NA,  
                             "PerfectSeperation" = NA)
  }
  tmp_results
}

@mayerbry
Copy link
Contributor

some tidyr workarounds (Jimmy confirmed worked):

control_testing_data %>%
  group_by(day_plot, measurement, sample_type) %>%
  summarise(x = list(pairwise_test_cont(x = pmax(magnitude, 86), group = study_grouped)),
               .groups = 'drop') %>%
  unnest(cols = x)

more explicit

control_testing_data %>%
  group_by(day_plot, measurement, sample_type) %>%
  summarise(x = list(pairwise_test_cont(x = pmax(magnitude, 86), group = study_grouped)),
               .groups = 'drop') %>%
  filter(!is.null(x)) %>%
  unnest(cols = x)

@wfulp
Copy link
Contributor Author

wfulp commented Sep 23, 2020

If a change is made a requirement is it being backwards compatible, specifically that it runs the same with nest, do, and group_modify functions

@wfulp wfulp added this to the 1.3.0 milestone Jun 17, 2021
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

No branches or pull requests

2 participants