Harmonize API #373

IndrajeetPatil · 2024-11-19T21:32:42Z

Rename args to follow color_* pattern

strengejacke · 2024-11-20T11:15:27Z

Looks good! Only conflict we have is that we have rope_alpha, but color_rope. It's difficult... :-/ @DominiqueMakowski Master of names, what do you think?

DominiqueMakowski · 2024-11-20T12:48:51Z

Can change for alpha_rope, it is aligned with the mental process of "I want to change the alpha of the rope"

strengejacke · 2024-11-20T12:53:32Z

So we (roughly) have the pattern aes_geom / aes_element...

bwiernik · 2024-11-20T16:01:12Z

One potential consideration is that ggplot2 names these sorts or arguments/aesthetics like: *.color

https://github.com/tidyverse/ggplot2/blob/main/R/geom-boxplot.R

We don't use . in argument names ever, so I don't think we should use the "full" ggplot2 naming scheme. Given that, I don't think it makes much difference whether we match the part-first order of names ggplot2 uses vs the aesthetic-first order used in this PR. Personally, I like aesthetic-first better and think we should use it. Just wanted to flag that it is a divergence from how ggplot2 and some extension packages name their arguments.

strengejacke · 2024-11-20T18:56:34Z

I don't think we cover this case here in our coding style guidelines: https://easystats.github.io/easystats/articles/conventions.html

strengejacke · 2024-11-20T20:51:44Z

What about the alphas?

library(easystats)
easystats_args <- NULL
for (pkg in easystats::easystats_packages()) {
  fns <- ls(paste0("package:", pkg))
  rds_filepath <- file.path(find.package(pkg), "NAMESPACE")
  
  all_fns <- tryCatch(
    as.data.frame(read.table(rds_filepath)),
    error = function(e){
      NULL
    }
  )
  if (!is.null(all_fns)) {
    names(all_fns) <- "func"
    all_fns <- data_filter(all_fns, startsWith(all_fns$func, "S3method("))
    fn <- gsub("S3method\\((.*)\\)", "\\1", all_fns$func)
    fn <- gsub(",", ".", fn, fixed = TRUE)
    
    all_args <- NULL
    for (i in fn) {
      all_args <- c(all_args, formalArgs(getFromNamespace(i, pkg)))
    }
    
    easystats_args <- c(easystats_args, sort(unique(all_args)))
  }
}

grep("alpha", sort(unique(easystats_args)), value = TRUE)
#> [1] "alpha"            "dispersion_alpha" "dot_alpha"        "line_alpha"      
#> [5] "posteriors_alpha" "priors_alpha"     "rope_alpha"       "si_alpha"

^{Created on 2024-11-20 with reprex v2.1.1}

bwiernik · 2024-11-20T20:56:53Z

alpha should come first like other aesthetics

bwiernik · 2024-11-20T20:57:44Z

(Those are all plotting parameters and not alpha as in significance right? I think we use "level" for the latter everywhere)

bwiernik · 2024-11-20T20:58:05Z

I don't think we cover this case here in our coding style guidelines: https://easystats.github.io/easystats/articles/conventions.html

Let's add that to ensure consistency going forward

strengejacke · 2024-11-20T20:58:48Z

(Those are all plotting parameters and not alpha as in significance right? I think we use "level" for the latter everywhere)

Yes, I think so.

strengejacke · 2024-11-20T21:04:10Z

library(easystats)
easystats_args <- NULL
for (pkg in easystats::easystats_packages()) {
  fns <- ls(paste0("package:", pkg))
  rds_filepath <- file.path(find.package(pkg), "NAMESPACE")
  
  # for some packages, read.table fails...
  all_fns <- tryCatch(
    as.data.frame(read.table(rds_filepath)),
    error = function(e){
      NULL
    }
  )
  if (!is.null(all_fns)) {
    names(all_fns) <- "func"
    all_fns <- data_filter(all_fns, startsWith(all_fns$func, "S3method("))
    fn <- gsub("S3method\\((.*)\\)", "\\1", all_fns$func)
    fn <- gsub(",", ".", fn, fixed = TRUE)
    
    all_args <- NULL
    for (i in fn) {
      fun_args <- formalArgs(getFromNamespace(i, pkg))
      all_args <- rbind(
        all_args,
        data.frame(args = fun_args, fun = i, pkg = pkg)
      )
    }
    easystats_args <- rbind(easystats_args, all_args)
  }
}

easystats_args[grepl("alpha", easystats_args$args, fixed = TRUE), ] |> export_table()
#> args             |                               fun | pkg
#> ----------------------------------------------------------
#> rope_alpha       |   plot.see_bayesfactor_parameters | see
#> rope_alpha       | plot.see_bayesfactor_savagedickey | see
#> alpha            |          plot.see_check_normality | see
#> dot_alpha        |          plot.see_check_normality | see
#> dot_alpha        |           plot.see_check_outliers | see
#> alpha            |          plot.see_check_residuals | see
#> dot_alpha        |          plot.see_check_residuals | see
#> rope_alpha       |         plot.see_equivalence_test | see
#> rope_alpha       |      plot.see_equivalence_test_df | see
#> rope_alpha       |      plot.see_equivalence_test_lm | see
#> priors_alpha     |         plot.see_estimate_density | see
#> posteriors_alpha |         plot.see_estimate_density | see
#> priors_alpha     |              plot.see_p_direction | see
#> line_alpha       |               plot.see_p_function | see
#> priors_alpha     |           plot.see_p_significance | see
#> posteriors_alpha |     plot.see_parameters_brms_meta | see
#> rope_alpha       |     plot.see_parameters_brms_meta | see
#> dispersion_alpha |  plot.see_parameters_distribution | see
#> posteriors_alpha |      plot.see_parameters_simulate | see
#> line_alpha       |     plot.see_performance_pp_check | see
#> alpha            |       plot.see_performance_simres | see
#> dot_alpha        |       plot.see_performance_simres | see
#> priors_alpha     |           plot.see_point_estimate | see
#> rope_alpha       |                     plot.see_rope | see
#> si_alpha         |                       plot.see_si | see
#> line_alpha       |    print.see_performance_pp_check | see

strengejacke · 2024-11-20T21:14:24Z

And we have all the sizes... base_size is maybe ok to keep, but what about font_size?

library(easystats)
easystats_args <- NULL
for (pkg in easystats::easystats_packages()) {
  fns <- ls(paste0("package:", pkg))
  rds_filepath <- file.path(find.package(pkg), "NAMESPACE")
  
  all_fns <- tryCatch(
    as.data.frame(read.table(rds_filepath)),
    error = function(e){
      NULL
    }
  )
  if (!is.null(all_fns)) {
    names(all_fns) <- "func"
    all_fns <- data_filter(all_fns, startsWith(all_fns$func, "S3method("))
    fn <- gsub("S3method\\((.*)\\)", "\\1", all_fns$func)
    fn <- gsub(",", ".", fn, fixed = TRUE)
    
    all_args <- NULL
    for (i in fn) {
      fun_args <- formalArgs(getFromNamespace(i, pkg))
      all_args <- rbind(
        all_args,
        data.frame(args = fun_args, fun = i, pkg = pkg)
      )
    }
    easystats_args <- rbind(easystats_args, all_args)
  }
}

easystats_args[grepl("size", easystats_args$args, fixed = TRUE), ] |>
  data_filter(pkg %in% c("see", "parameters")) |> 
  export_table()
#> args            |                                 fun |        pkg
#> ------------------------------------------------------------------
#> font_size       |          display.compare_parameters | parameters
#> font_size       |        display.parameters_brms_meta | parameters
#> font_size       |            display.parameters_model | parameters
#> font_size       |         display.parameters_simulate | parameters
#> font_size       |       print_html.compare_parameters | parameters
#> font_size       |     print_html.parameters_brms_meta | parameters
#> font_size       |         print_html.parameters_model | parameters
#> font_size       |           print_html.parameters_sem | parameters
#> font_size       |      print_html.parameters_simulate | parameters
#> size_point      |     plot.see_bayesfactor_parameters |        see
#> size_point      |   plot.see_bayesfactor_savagedickey |        see
#> size_line       |           plot.see_binned_residuals |        see
#> size_point      |           plot.see_binned_residuals |        see
#> size_title      |           plot.see_binned_residuals |        see
#> size_axis_title |           plot.see_binned_residuals |        see
#> base_size       |           plot.see_binned_residuals |        see
#> size_point      |         plot.see_check_collinearity |        see
#> size_line       |         plot.see_check_collinearity |        see
#> size_title      |         plot.see_check_collinearity |        see
#> size_axis_title |         plot.see_check_collinearity |        see
#> base_size       |         plot.see_check_collinearity |        see
#> size_point      |                  plot.see_check_dag |        see
#> size_text       |                  plot.see_check_dag |        see
#> size_point      |         plot.see_check_distribution |        see
#> size_point      | plot.see_check_distribution_numeric |        see
#> size_point      |   plot.see_check_heteroscedasticity |        see
#> size_line       |   plot.see_check_heteroscedasticity |        see
#> size_title      |   plot.see_check_heteroscedasticity |        see
#> size_axis_title |   plot.see_check_heteroscedasticity |        see
#> base_size       |   plot.see_check_heteroscedasticity |        see
#> size_line       |            plot.see_check_normality |        see
#> size_point      |            plot.see_check_normality |        see
#> size_title      |            plot.see_check_normality |        see
#> size_axis_title |            plot.see_check_normality |        see
#> base_size       |            plot.see_check_normality |        see
#> size_text       |             plot.see_check_outliers |        see
#> size_line       |             plot.see_check_outliers |        see
#> size_title      |             plot.see_check_outliers |        see
#> size_axis_title |             plot.see_check_outliers |        see
#> base_size       |             plot.see_check_outliers |        see
#> size_line       |             plot.see_check_overdisp |        see
#> size_title      |             plot.see_check_overdisp |        see
#> size_axis_title |             plot.see_check_overdisp |        see
#> base_size       |             plot.see_check_overdisp |        see
#> size_line       |            plot.see_check_residuals |        see
#> size_point      |            plot.see_check_residuals |        see
#> size_title      |            plot.see_check_residuals |        see
#> size_axis_title |            plot.see_check_residuals |        see
#> base_size       |            plot.see_check_residuals |        see
#> size_point      |         plot.see_compare_parameters |        see
#> size_text       |         plot.see_compare_parameters |        see
#> size_line       |        plot.see_compare_performance |        see
#> size_point      |        plot.see_equivalence_test_lm |        see
#> size_line       |           plot.see_estimate_density |        see
#> size_point      |           plot.see_estimate_density |        see
#> size_line       |        plot.see_estimate_density_df |        see
#> size            |                 plot.see_n_clusters |        see
#> size            |                  plot.see_n_factors |        see
#> size_point      |                 plot.see_p_function |        see
#> size_line       |                 plot.see_p_function |        see
#> size_text       |                 plot.see_p_function |        see
#> size_point      |       plot.see_parameters_brms_meta |        see
#> size_line       |       plot.see_parameters_brms_meta |        see
#> size_text       |       plot.see_parameters_brms_meta |        see
#> size_bar        |    plot.see_parameters_distribution |        see
#> size_text       |             plot.see_parameters_efa |        see
#> size            |             plot.see_parameters_efa |        see
#> size_point      |           plot.see_parameters_model |        see
#> size_text       |           plot.see_parameters_model |        see
#> size_text       |             plot.see_parameters_pca |        see
#> size            |             plot.see_parameters_pca |        see
#> size_point      |             plot.see_parameters_sem |        see
#> size_line       |        plot.see_parameters_simulate |        see
#> size_line       |       plot.see_performance_pp_check |        see
#> size_point      |       plot.see_performance_pp_check |        see
#> size_bar        |       plot.see_performance_pp_check |        see
#> size_axis_title |       plot.see_performance_pp_check |        see
#> size_title      |       plot.see_performance_pp_check |        see
#> base_size       |       plot.see_performance_pp_check |        see
#> size_line       |         plot.see_performance_simres |        see
#> size_point      |         plot.see_performance_simres |        see
#> size_title      |         plot.see_performance_simres |        see
#> size_axis_title |         plot.see_performance_simres |        see
#> base_size       |         plot.see_performance_simres |        see
#> size_point      |             plot.see_point_estimate |        see
#> size_text       |             plot.see_point_estimate |        see
#> size_line       |      print.see_performance_pp_check |        see
#> size_point      |      print.see_performance_pp_check |        see
#> size_bar        |      print.see_performance_pp_check |        see
#> size_axis_title |      print.see_performance_pp_check |        see
#> size_title      |      print.see_performance_pp_check |        see
#> base_size       |      print.see_performance_pp_check |        see

bwiernik · 2024-11-20T21:27:42Z

size_line we should change to linewidth since that's the correct ggplot2 argument now

bwiernik · 2024-11-20T21:28:09Z

Let's leave font_size

strengejacke · 2024-11-22T14:11:33Z

I think we should change the arguments in check_model(), too, to align with the plot-functions?
https://easystats.github.io/performance/reference/check_model.html

bwiernik · 2024-11-22T14:18:41Z

Yep absolutely. Are there other functions that also have plotting arguments?

strengejacke · 2024-11-22T15:24:47Z

I don't think so. It's because check_model does only plotting, you don't need to plot() explicitly. that's why you be pass those arguments directly in check_model. Maybe binned_residuals or check_predictions, I'll check

bwiernik

There are a couple of places where we say "color" to refer fill type variables. Should we switch to "fill_" for those? I'm leaning toward no, as we use one color argument to set both fill and color aesthetics in several places

bwiernik · 2024-11-22T14:30:48Z

R/plot.describe_distribution.R

-#' @param dispersion_alpha Numeric value specifying the transparency level of dispersion ribbon.
-#' @param dispersion_color Character specifying the color of dispersion ribbon.
+#' @param alpha_dispersion Numeric value specifying the transparency level of dispersion ribbon.
+#' @param color_dispersion Character specifying the color of dispersion ribbon.


Should this be fill_dispersion?

bwiernik · 2024-11-22T14:31:43Z

R/plot.describe_distribution.R

 #' @param dispersion_style Character describing the style of dispersion area.
 #'   `"ribbon"` for a ribbon, `"curve"` for a normal-curve.
 #' @param highlight A vector with names of categories in `x` that should be
 #'   highlighted.
-#' @param highlight_color A vector of color values for highlighted categories.
+#' @param color_highlight A vector of color values for highlighted categories.


Should this be fill_highlight?

bwiernik · 2024-11-22T14:32:56Z

R/plot.dw_data_tabulate.R

@@ -12,7 +12,7 @@
 #'   If `TRUE`, confidence intervals computed using the Wilson method are shown.
 #'   See Brown et al. (2001) for details.
 #' @param ci Confidence Interval (CI) level. Defaults to `0.95` (`95%`).
-#' @param fill_col Color to use for category columns (default: `"#87CEFA"`).
+#' @param color_fill Color to use for category columns (default: `"#87CEFA"`).


Should this be fill_bar or just fill?

strengejacke · 2024-11-23T08:05:21Z

There are a couple of places where we say "color" to refer fill type variables. Should we switch to "fill_" for those? I'm leaning toward no, as we use one color argument to set both fill and color aesthetics in several places

I think we don't need to fully adopt the ggplot2 aes names. If it's a color aesthetic, no matter if for points, lines, or ribbons, color_* is ok.

strengejacke · 2024-11-23T08:06:52Z

size_line we should change to linewidth since that's the correct ggplot2 argument now

I'm fine with both, we could also keep size_* for anything that changes geom sizes in a certain way

IndrajeetPatil added 3 commits November 19, 2024 22:32

Rename arg to follow color_* pattern

14238e0

tun tests conditionally

7b1e30f

assume vdiffr is always available

779588a

IndrajeetPatil added 2 commits November 21, 2024 21:44

size_line -> linewidth

21a9a9a

rename parameters to alpha_* pattern

4c09de2

IndrajeetPatil marked this pull request as ready for review November 21, 2024 20:55

IndrajeetPatil requested review from strengejacke and bwiernik November 21, 2024 20:55

bwiernik reviewed Nov 22, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Harmonize API #373

Harmonize API #373

IndrajeetPatil commented Nov 19, 2024

strengejacke commented Nov 20, 2024

DominiqueMakowski commented Nov 20, 2024

strengejacke commented Nov 20, 2024

bwiernik commented Nov 20, 2024

strengejacke commented Nov 20, 2024

strengejacke commented Nov 20, 2024

bwiernik commented Nov 20, 2024

bwiernik commented Nov 20, 2024

bwiernik commented Nov 20, 2024

strengejacke commented Nov 20, 2024

strengejacke commented Nov 20, 2024 •

edited

Loading

strengejacke commented Nov 20, 2024

bwiernik commented Nov 20, 2024

bwiernik commented Nov 20, 2024

strengejacke commented Nov 22, 2024

bwiernik commented Nov 22, 2024

strengejacke commented Nov 22, 2024

bwiernik left a comment

bwiernik Nov 22, 2024

bwiernik Nov 22, 2024

bwiernik Nov 22, 2024

strengejacke commented Nov 23, 2024

strengejacke commented Nov 23, 2024

Harmonize API #373

Are you sure you want to change the base?

Harmonize API #373

Conversation

IndrajeetPatil commented Nov 19, 2024

strengejacke commented Nov 20, 2024

DominiqueMakowski commented Nov 20, 2024

strengejacke commented Nov 20, 2024

bwiernik commented Nov 20, 2024

strengejacke commented Nov 20, 2024

strengejacke commented Nov 20, 2024

bwiernik commented Nov 20, 2024

bwiernik commented Nov 20, 2024

bwiernik commented Nov 20, 2024

strengejacke commented Nov 20, 2024

strengejacke commented Nov 20, 2024 • edited Loading

strengejacke commented Nov 20, 2024

bwiernik commented Nov 20, 2024

bwiernik commented Nov 20, 2024

strengejacke commented Nov 22, 2024

bwiernik commented Nov 22, 2024

strengejacke commented Nov 22, 2024

bwiernik left a comment

Choose a reason for hiding this comment

bwiernik Nov 22, 2024

Choose a reason for hiding this comment

bwiernik Nov 22, 2024

Choose a reason for hiding this comment

bwiernik Nov 22, 2024

Choose a reason for hiding this comment

strengejacke commented Nov 23, 2024

strengejacke commented Nov 23, 2024

strengejacke commented Nov 20, 2024 •

edited

Loading