Standardize group arguments in functions #105

EmilHvitfeldt · 2023-05-09T02:32:51Z

This PR aims to tackle part of #104. Change 1 specifically.

While it might seems strange, I'm sticking with group over .by, as it is consistent with the existing codebase. Once this is done, I will swap everything over to .by as part of Change 2.

add group argument to cal_estimate_*.data.frame() functions
add group argument to cal_plot_.*.data.frame() functions
Handle .config in call_estimate_*.tune_results()` functions
Handle .config in call_plot_*.tune_results()` functions

EmilHvitfeldt · 2023-05-10T01:35:41Z

R/cal-estimate-beta.R

+                                         ...,
+                                         group = NULL) {
  stop_null_parameters(parameters)
+
+  check_group_argument({{ group }}, .data)
+  .data <- dplyr::group_by(.data, dplyr::across({{ group }}))
+


All *.data.frame() functions calls check_group_argument() and applies dplyr::group_by() with the group. This does nothing if group = NULL

EmilHvitfeldt · 2023-05-10T01:36:53Z

R/cal-plot-breaks.R

-    group <- expr(.config)
-  }
-
-  cal_plot_breaks_impl(
+  tune_args <- tune_results_args(
    .data = .data,
    truth = {{ truth }},
    estimate = {{ estimate }},
-    group = {{ group }},
+    event_level = event_level,
+    ...
+  )
+
+  cal_plot_breaks_impl(
+    .data = tune_args$predictions,
+    truth = !!tune_args$truth,
+    estimate = !!tune_args$estimate,
+    group = !!tune_args$group,


All *tune.results() are changed to use tune_results_args().

EmilHvitfeldt · 2023-05-10T01:38:24Z

R/cal-plot-regression.R

+  if (quo_is_null(group)) {
+    .data[[".config"]] <- NULL
+  }


If there isn't a group variable, e.i. .config is constant, it is removed from the plotting data.

This makes it easier to test that faceting isn't done with constant .config

EmilHvitfeldt · 2023-05-10T01:39:03Z

R/cal-plot-utils.R

+  if (dplyr::n_distinct(.data[[".predictions"]][[1]][[".config"]]) > 1) {
    group <- quo(.config)
+  } else {
+    group <- quo(NULL)
  }


this is the part that detects if there are more than 1 unique .config

topepo · 2023-05-10T17:17:41Z

For multinomial outcomes, much (but not all) of the infrastructure make a series of 1:all problems and applies binomial methods.

While the automatic detection of .config works for binomial, it doesn't find it for multinomial:

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"



library(tidymodels)
library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered

tidymodels_prefer()
theme_set(theme_bw())
options(pillar.advice = FALSE, pillar.min_title_chars = Inf)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)

^{Created on 2023-05-10 with reprex v2.0.2}

(using group = .config does work though).

topepo · 2023-05-10T17:17:58Z

For multinomial outcomes, much (but not all) of the infrastructure make a series of 1:all problems and applies binomial methods.

While the automatic detection of .config works for binomial, it doesn't find it for multinomial:

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"



library(tidymodels)
library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered

tidymodels_prefer()
theme_set(theme_bw())
options(pillar.advice = FALSE, pillar.min_title_chars = Inf)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)

^{Created on 2023-05-10 with reprex v2.0.2}

(using group = .config does work though).

topepo · 2023-05-10T17:19:02Z

For multinomial outcomes, much (but not all) of the infrastructure make a series of 1:all problems and applies binomial methods.

While the automatic detection of .config works for binomial, it doesn't find it for multinomial:

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"



library(tidymodels)
library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered

tidymodels_prefer()
theme_set(theme_bw())
options(pillar.advice = FALSE, pillar.min_title_chars = Inf)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)

^{Created on 2023-05-10 with reprex v2.0.2}

(using group = .config does work though).

topepo · 2023-05-10T17:20:13Z

For multinomial outcomes, much (but not all) of the infrastructure make a series of 1:all problems and applies binomial methods.

While the automatic detection of .config works for binomial, it doesn't find it for multinomial:

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)
#> Error in hpc_cv %>% mutate(.config = sample(letters[1:2], nrow(hpc_cv), : could not find function "%>%"



library(tidymodels)
library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered

tidymodels_prefer()
theme_set(theme_bw())
options(pillar.advice = FALSE, pillar.min_title_chars = Inf)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)

^{Created on 2023-05-10 with reprex v2.0.2}

(using group = .config does work though).

if group is null and .config is present, return .config

EmilHvitfeldt · 2023-05-11T20:44:22Z

That should be fixed now

library(tidymodels)
library(probably)
#> 
#> Attaching package: 'probably'
#> The following objects are masked from 'package:base':
#> 
#>     as.factor, as.ordered

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_windowed(truth = obs, VF:L)

hpc_cv %>%
  mutate(.config = sample(letters[1:2], nrow(hpc_cv), replace = TRUE)) %>% 
  cal_plot_breaks(truth = obs, VF:L)

^{Created on 2023-05-11 with reprex v2.0.2}

github-actions · 2023-05-26T00:41:39Z

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.

EmilHvitfeldt and others added 14 commits May 8, 2023 17:46

add check_group_argument()

86a0f0d

add group argument to all cal_estimate_*() functions

cc4a9c9

only allow a single col in groups

78b62cb

test groups argument of cal_estimate_*() functions

5ba17f5

use check_group_argument() in cal_plot_*() functions

044125a

remove tests that no longer pass

6d7b5aa

add tests for cal_plot_*() functions

5726549

update snapshots

99245e2

use .config as group

550fc5a

correctly detect number of unique .config

00ddfe3

use tune_results_args() in all plot functions

adbda5d

add tests for single .config plots

d356a1b

remove group argument in tune_results_args()

65fe163

remove group argument from cal_plot_*.tune_results() functions

e7e7b2c

EmilHvitfeldt commented May 10, 2023

View reviewed changes

EmilHvitfeldt requested a review from topepo May 10, 2023 01:39

topepo and others added 7 commits May 10, 2023 14:38

more tests for groups/configs

e992560

more tests for groups/configs in estimation

9627748

make new helpers runable

04d41fc

check_group_argument -> get_group_argument

b4053ea

if group is null and .config is present, return .config

update tests

fd6c045

add testthat_cal_multiclass_count()

639d8b1

use testthat_cal_multiclass_count() in tests

16760a5

topepo approved these changes May 12, 2023

View reviewed changes

topepo merged commit 88ef667 into main May 12, 2023

topepo deleted the groups branch May 12, 2023 00:04

github-actions bot locked and limited conversation to collaborators May 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standardize group arguments in functions #105

Standardize group arguments in functions #105

EmilHvitfeldt commented May 9, 2023 •

edited

Loading

EmilHvitfeldt May 10, 2023

EmilHvitfeldt May 10, 2023

EmilHvitfeldt May 10, 2023

EmilHvitfeldt May 10, 2023

topepo commented May 10, 2023

topepo commented May 10, 2023

topepo commented May 10, 2023

topepo commented May 10, 2023

EmilHvitfeldt commented May 11, 2023

github-actions bot commented May 26, 2023

Standardize group arguments in functions #105

Standardize group arguments in functions #105

Conversation

EmilHvitfeldt commented May 9, 2023 • edited Loading

EmilHvitfeldt May 10, 2023

Choose a reason for hiding this comment

EmilHvitfeldt May 10, 2023

Choose a reason for hiding this comment

EmilHvitfeldt May 10, 2023

Choose a reason for hiding this comment

EmilHvitfeldt May 10, 2023

Choose a reason for hiding this comment

topepo commented May 10, 2023

topepo commented May 10, 2023

topepo commented May 10, 2023

topepo commented May 10, 2023

EmilHvitfeldt commented May 11, 2023

github-actions bot commented May 26, 2023

EmilHvitfeldt commented May 9, 2023 •

edited

Loading