ggcoef() redesign #392

larmarange · 2020-10-18T12:32:44Z

Following #372 this PR proposes a complete redesign of ggcoef() based on broom.helpers package (also now used by gtsummary::tbl_regression()).

The PR includes 3 main functions: ggcoef_model(), ggcoef_compare() and ggcoef_multinom(). The original ggcoef() remains unchanged.

The PR also includes two new geoms: geom_stripped_rows() and geom_stripped_cols().

Pre-requisite before merging this PR:

PR Improvements and bug fixes for ggally_colbar(), ggally_rowbar() and ggbivariate() #375 merged in master
broom.helpers version 1.1.0 released on CRAN
unit tests added
vignette added

and better examples not relying on gtsummary::trial

DESCRIPTION

codecov-io · 2020-10-19T16:17:01Z

Codecov Report

Merging #392 (41c4e90) into master (f289584) will increase coverage by 0.20%.
The diff coverage is 94.98%.

@@            Coverage Diff             @@
##           master     #392      +/-   ##
==========================================
+ Coverage   92.05%   92.25%   +0.20%     
==========================================
  Files          34       36       +2     
  Lines        4054     4353     +299     
==========================================
+ Hits         3732     4016     +284     
- Misses        322      337      +15

Impacted Files	Coverage Δ
R/gg-plots.R	`82.88% <ø> (ø)`
R/ggcoef.R	`82.97% <ø> (ø)`
R/gglyph.R	`100.00% <ø> (ø)`
R/ggnet.R	`99.64% <ø> (ø)`
R/ggnet2.R	`99.03% <ø> (ø)`
R/ggnetworkmap.R	`86.53% <ø> (ø)`
R/ggnostic.R	`96.48% <ø> (ø)`
R/ggpairs_add.R	`94.40% <ø> (ø)`
R/stat_cross.R	`98.70% <ø> (ø)`
R/stat_prop.R	`68.65% <ø> (ø)`
... and 5 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f289584...af9b1e8. Read the comment docs.

Merge branch 'master' of github.com:ggobi/ggally into ggcoef_model # Conflicts: # DESCRIPTION

…ef_model

ddsjoberg · 2020-10-30T16:37:19Z

Hello @larmarange ! I was playing around with ggcoef_model() and ggcoef_plot() this morning, and they look fantastic. I wanted to share a couple of thoughts I had while using them.

Perhaps a better default for ggcoef_model(significance = 0.05) would be ggcoef_model(significance = 1 - conf.level)?
ggcoef_plot() does not have the same significance labelling ability as ggcoef_model(significance=, significance_labels=, show_p_values=, signif_stars=). Is there a reason they were omitted? Of course, the significance level/confidence level cannot be inferred from a table, but if a user manually supplied the level along with the tidy_plus_plus() tibble I don't think there would be an issue.
For categorical variables, I love the default labelling. Is there a way, however, to avoid the duplicate variable labels for the continuous variables in the models?
```
mod <- lm(ttdeath ~ marker + trt, gtsummary::trial)
ggcoef_model(mod)
```

larmarange · 2020-10-30T18:03:21Z

Hello @larmarange ! I was playing around with ggcoef_model() and ggcoef_plot() this morning, and they look fantastic. I wanted to share a couple of thoughts I had while using them.

Thanks a lot for your feedback

Perhaps a better default for ggcoef_model(significance = 0.05) would be ggcoef_model(significance = 1 - conf.level)?

Good idea. Implemented through f5743c8

ggcoef_plot() does not have the same significance labelling ability as ggcoef_model(significance=, significance_labels=, show_p_values=, signif_stars=). Is there a reason they were omitted? Of course, the significance level/confidence level cannot be inferred from a table, but if a user manually supplied the level along with the tidy_plus_plus() tibble I don't think there would be an issue.

This is a choice by design. ggcoef_model() only have graphic options for advanced users. If a user want to create is own "significance" variable, this variable could be mapped using the shape argument of ggcoef_model().

library(GGally)
#> Le chargement a nécessité le package : ggplot2
#> Registered S3 method overwritten by 'GGally':
#>   method from   
#>   +.gg   ggplot2
library(broom.helpers)
mod <- lm(Sepal.Length ~ ., iris)
res <- mod %>% 
  tidy_plus_plus() %>%
  dplyr::mutate(stars = signif_stars(p.value))
ggcoef_plot(res, shape = "stars", shape_values = 16:19)

As you can see, any custom variable could be mapped to shape.

For categorical variables, I love the default labelling. Is there a way, however, to avoid the duplicate variable labels for the continuous variables in the models?
mod <- lm(ttdeath ~ marker + trt, gtsummary::trial)
ggcoef_model(mod)

This is a good question. One label is needed for facets (the bold ones are in fact labels) and one is required for y-axis.

I guess I found a solution implemented in 732f5fd

library(GGally)
#> Le chargement a nécessité le package : ggplot2
#> Registered S3 method overwritten by 'GGally':
#>   method from   
#>   +.gg   ggplot2
library(broom.helpers)
mod <- lm(Sepal.Length ~ ., iris)
ggcoef_model(mod)

ggcoef_model(mod, group_one_row_variables = TRUE)

mod1 <- lm(Fertility ~ ., data = swiss)
mod2 <- step(mod1, trace = 0)
mod3 <- lm(Fertility ~ Agriculture + Education * Catholic, data = swiss)
models <- list("Full model" = mod1, "Simplified model" = mod2, "With interaction" = mod3)
ggcoef_compare(models)

ggcoef_compare(models, group_one_row_variables = TRUE)

^{Created on 2020-10-30 by the reprex package (v0.3.0)}

ddsjoberg · 2020-10-30T18:15:07Z

This is all great! I am thinking I can pass tbl_regression()$table_body %>% ggcoef_plot() to get a great visual of the tabulated model. 🕺

larmarange · 2020-10-31T10:35:44Z

@ddsjoberg If you want to pass tbl_regression()$table_body, don't forget to remove header rows who are not expected by ggcoef_plot()

larmarange · 2020-11-01T09:32:57Z

What should be the rule when p values are printed on y levels? Should they stay on y axis or be moved to the facet label? Joseph Larmarange Le sam. 31 oct. 2020 à 13:26, Daniel Sjoberg <notifications@github.com> a écrit :

…

I like the facetted grouping of the variables. I think it is quite visually appealing. Perhaps something like this could work, where the y-axis label is omitted when it is re-printing the variable label? I know there is a lot going on in the background, so this may not be possible (I see this messes with the striping). Just wanted to share some thoughts...this is your baby and you know best :) library(GGally) lm(Sepal.Length ~ ., iris) %>% broom.helpers::tidy_plus_plus() %>% dplyr::mutate(label = ifelse(var_type == "continuous", " ", label)) %>% ggcoef_plot() <https://camo.githubusercontent.com/df1a302cdcff5de97b53143555f4160067b128be/68747470733a2f2f692e696d6775722e636f6d2f624b4d596b624a2e706e67> Created on 2020-10-31 by the reprex package <https://reprex.tidyverse.org> (v0.3.0) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#392 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAHL5I4WZOQT5M6S5TXJPUTSNP67XANCNFSM4SVBX7BQ> .

larmarange · 2020-11-01T12:14:54Z

@ddsjoberg I have implemented an alternative to group_one_row_variables.

The functions compute a label_light where variable label is removed when identical to term label. It is still possible to map the full label if needed.

larmarange · 2020-11-01T12:16:39Z

library(GGally)
#> Le chargement a nécessité le package : ggplot2
#> Registered S3 method overwritten by 'GGally':
#> method from
#> +.gg ggplot2
library(broom.helpers)
mod <- lm(Sepal.Length ~ ., iris)
ggcoef_model(mod)

ggcoef_model(mod, y = "label")

ggcoef_model(mod, facet_row = NULL)

mod1 <- lm(Fertility ~ ., data = swiss)
mod2 <- step(mod1, trace = 0)
mod3 <- lm(Fertility ~ Agriculture + Education * Catholic, data = swiss)
models <- list("Full model" = mod1, "Simplified model" = mod2, "With interaction" = mod3)
ggcoef_compare(models)

Created on 2020-11-01 by the reprex package (v0.3.0)

ddsjoberg · 2020-11-01T14:03:48Z

@larmarange I think these plots look fantastic!

ddsjoberg · 2020-11-01T14:19:32Z

@larmarange one more question. Is it possible to get this kind of output from ggcoef_plot() with the proper striping?

library(GGally)

lm(Sepal.Length ~ ., iris) %>%
  broom.helpers::tidy_plus_plus() %>%
  dplyr::mutate(label = ifelse(var_type == "continuous", " ", label)) %>%
  ggcoef_plot()

^{Created on 2020-10-31 by the reprex package (v0.3.0)}

larmarange · 2020-11-01T15:02:53Z

@larmarange one more question. Is it possible to get this kind of output from ggcoef_plot() with the proper striping?
library(GGally)

lm(Sepal.Length ~ ., iris) %>%
  broom.helpers::tidy_plus_plus() %>%
  dplyr::mutate(label = ifelse(var_type == "continuous", " ", label)) %>%
  ggcoef_plot()

The problem here is that due to the relabel, all empty y values have the same value (in terms of factor) and therefore have the same strip.

larmarange · 2020-11-01T15:22:11Z

I have added a little trick using term column for computing strips colour.

library(GGally)
#> Le chargement a nécessité le package : ggplot2
#> Registered S3 method overwritten by 'GGally':
#>   method from   
#>   +.gg   ggplot2

lm(Sepal.Length ~ ., iris) %>%
  broom.helpers::tidy_plus_plus() %>%
  dplyr::mutate(label = ifelse(var_type == "continuous", "", label)) %>%
  ggcoef_plot()

Note: your assumption dplyr::mutate(label = ifelse(var_type == "continuous", " ", label) is not always correct for continuous variables using stats::poly (in that case you ave several terms for one continuous variable).

ddsjoberg · 2020-11-01T15:51:09Z

These figures are amazing! Thank you for your work on them!

larmarange · 2020-11-24T08:55:55Z

broom.helpers version 1.1.0 has just been released on CRAN. We have to wait for MacOS and Windows binaries to re-run all tests and then the PR should be finally ready. :-)

larmarange · 2020-11-27T12:58:20Z

Hi @schloerke

broom.helpers v1.1.0 is now on CRAN and windows binaries have been generated.

This PR is now ready for being merged.

Only one check failed (ubuntu 16.04 oldrel) but it seems unrelated to the PR. It fails with the installation of 'r-hub/sysreqs'.

Best regards

schloerke · 2020-11-30T15:10:32Z

@larmarange Will look through this later tonight. Thank you!

larmarange · 2020-11-30T16:07:32Z

Thanks to you @schloerke

schloerke

Minor changes.

Great code! Thank you for awesome formatting. It was very easy to read! Your ggplot2 integrations are getting / have been beyond my working knowledge. 😳

The vignette looks great! Thank you!

R/ggcoef_model.R

DESCRIPTION

R/ggcoef_model.R

tests/testthat/test-ggcoef_model.R

R/geom_stripped_rows.R

R/ggcoef_model.R

vignettes/ggcoef_model.Rmd

R/ggcoef_model.R

Co-authored-by: Barret Schloerke <barret@rstudio.com>

larmarange · 2020-12-02T08:27:53Z

@schloerke

Thank you very much for your careful review. I have merged your proposed corrections.

See my answer regarding xfom and xto.

Best regards

fix ggobi#391

fix #391

larmarange and others added 11 commits October 4, 2020 21:16

first draft of ggcoef_model

fa38f39

follow broom.helpers::tidy_select_variables()

970571e

keep parameter for ggcoef_compare

023674a

follow broom.helpers

5783a58

labellers arguments and minor improvements

2ca5b57

bug fixes

4e7799e

add categorical_terms_pattern

3b4336e

add the possibility to control row facets

f9b61ee

and better examples not relying on gtsummary::trial

update doc

3a045fb

geom_stripped_rows should inherits y

d225171

Merge branch 'master' into ggcoef_model

83910c0

schloerke reviewed Oct 19, 2020

View reviewed changes

DESCRIPTION Show resolved Hide resolved

schloerke and others added 9 commits October 19, 2020 12:18

Add remote to broom.helpers

f18a17a

merge with current master

bfe4fcc

Merge branch 'master' of github.com:ggobi/ggally into ggcoef_model # Conflicts: # DESCRIPTION

update documentation

7490347

Merge branch 'ggcoef_model' of github.com:larmarange/ggally into ggco…

08e2f06

…ef_model

CHECK fixes

8880cf3

correct syntax for tidyr::nesting()

1785707

missing topics for pkgdown

0bb4ba7

fixing documentation links

c56ac72

update ggcoef_model doc

aec4799

larmarange added 2 commits October 30, 2020 18:23

change default value of significance

f5743c8

nex option group_one_row_variables

732f5fd

improve group_one_row_variables option

a700b89

some tests for ggcoef_model

6cc0c67

alternative to group_one_row_variables

025c0f2

larmarange added 2 commits November 1, 2020 15:42

test update

61f766c

import stringr

5b20bec

trick for correct strips

851150c

larmarange added 2 commits November 24, 2020 09:42

broom.helpers 1.1.0 now on CRAN

057ce3e

classic ggplot2 functions in the vignette

012f996

minor change to force rechecks

7f3b2a8

typo

41c4e90

schloerke requested changes Dec 2, 2020

View reviewed changes

schloerke reviewed Dec 2, 2020

View reviewed changes

R/ggcoef_model.R Outdated Show resolved Hide resolved

larmarange and others added 3 commits December 2, 2020 09:04

Apply suggestions from code review

af9b1e8

Co-authored-by: Barret Schloerke <barret@rstudio.com>

document

23824eb

force dplyr

cd0e483

schloerke merged commit 9de5508 into ggobi:master Dec 2, 2020

schloerke mentioned this pull request Dec 11, 2020

stat_cross() and ggtable() broken since broom v7.0.0 #391

Closed

larmarange added a commit to larmarange/ggally that referenced this pull request Dec 11, 2020

NEWS update about ggobi#392

5f647ff

fix ggobi#391

schloerke pushed a commit that referenced this pull request Dec 11, 2020

NEWS update about #392

e40ae14

fix #391

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ggcoef() redesign #392

ggcoef() redesign #392

larmarange commented Oct 18, 2020 •

edited

Loading

codecov-io commented Oct 19, 2020 •

edited

Loading

ddsjoberg commented Oct 30, 2020

larmarange commented Oct 30, 2020

ddsjoberg commented Oct 30, 2020

larmarange commented Oct 31, 2020

larmarange commented Nov 1, 2020 via email

larmarange commented Nov 1, 2020

larmarange commented Nov 1, 2020

ddsjoberg commented Nov 1, 2020

ddsjoberg commented Nov 1, 2020

larmarange commented Nov 1, 2020

larmarange commented Nov 1, 2020

ddsjoberg commented Nov 1, 2020

larmarange commented Nov 24, 2020

larmarange commented Nov 27, 2020

schloerke commented Nov 30, 2020

larmarange commented Nov 30, 2020

schloerke left a comment

larmarange commented Dec 2, 2020

ggcoef() redesign #392

ggcoef() redesign #392

Conversation

larmarange commented Oct 18, 2020 • edited Loading

codecov-io commented Oct 19, 2020 • edited Loading

Codecov Report

ddsjoberg commented Oct 30, 2020

larmarange commented Oct 30, 2020

ddsjoberg commented Oct 30, 2020

larmarange commented Oct 31, 2020

larmarange commented Nov 1, 2020 via email

larmarange commented Nov 1, 2020

larmarange commented Nov 1, 2020

ddsjoberg commented Nov 1, 2020

ddsjoberg commented Nov 1, 2020

larmarange commented Nov 1, 2020

larmarange commented Nov 1, 2020

ddsjoberg commented Nov 1, 2020

larmarange commented Nov 24, 2020

larmarange commented Nov 27, 2020

schloerke commented Nov 30, 2020

larmarange commented Nov 30, 2020

schloerke left a comment

Choose a reason for hiding this comment

larmarange commented Dec 2, 2020

larmarange commented Oct 18, 2020 •

edited

Loading

codecov-io commented Oct 19, 2020 •

edited

Loading