Add n

[larmarange]

Exploratory and experimental PR

Not to be considered for merging at this stage

• add tests for a wide range of models
• integration in tidy_plus_plus()
• finalize documentation
• finalize examples in documentation
• update vignette
• update NEWS

[larmarange]

Dear @ddsjoberg

tidy_add_n() is ready for review if you want to test it.

Cheers

[ddsjoberg]

Dear @ddsjoberg

tidy_add_n() is ready for review if you want to test it.

Cheers

Awesome! I'll take a look!

[ddsjoberg]

@larmarange I've been playing around with this and it's perfect. I'll do some more testing. The only thing I would add is more details to the description "Add the number of observations in a new column n taking into account interaction terms and the different types of contrasts." I would add what numbers it adds for continuous, categorical, and dichotomous data. What will be reported for various contrasts, etc.

[larmarange]

Dear @ddsjoberg

I have developed the documentation of tidy_add_n(). I'm not sure if it is clear and I would appreciate an external look at it.

Best

[larmarange]

Small question: what should be the default value for add_n in tidy_plus_plus()? TRUE or FALSE? As it is not changing the structure of the results (just additional columns), it's maybe worth to be TRUE by default (for simplicity for final users)?

[ddsjoberg]

Small question: what should be the default value for add_n in tidy_plus_plus()? TRUE or FALSE? As it is not changing the structure of the results (just additional columns), it's maybe worth to be TRUE by default (for simplicity for final users)?

I think that is a great idea! I don't think it would cause any issues downstream.

[ddsjoberg]

hey hey, the update looks fantastic! I tried to break it in various ways, and it always performed perfectly.

There is just one thing I am requesting to be changed. The new columns added "n" and "nevent" are used in gtsummary already after add_nevent(). I know that users go into .$table_body and futz with these columns, and it would be helpful to avoid changes that break their code. What do you think about "nobs" and "nobs_event" (or anything else really!)?

[larmarange]

There is just one thing I am requesting to be changed. The new columns added "n" and "nevent" are used in gtsummary already after add_nevent(). I know that users go into .$table_body and futz with these columns, and it would be helpful to avoid changes that break their code. What do you think about "nobs" and "nobs_event" (or anything else really!)?

Do you think they could be a confusion between the columns created by broom.helpers and those created by gtsummary::add_nevent()? Somehow, is it redundant?

We can change the names in broom.helpers. Would maybe prefer n_obs and n_event.

Regards

[codecov]

Codecov Report

Merging #64 (dfd1852) into master (0ba57c4) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##            master       #64    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files           27        33     +6     
  Lines         1154      1336   +182     
==========================================
+ Hits          1154      1336   +182     

Impacted Files	Coverage Δ
R/helpers.R	100.00% <ø> (ø)
R/broom.helpers-package.R	100.00% <100.00%> (ø)
R/model_compute_terms_contributions.R	100.00% <100.00%> (ø)
R/model_get_model_matrix.R	100.00% <100.00%> (ø)
R/model_get_n.R	100.00% <100.00%> (ø)
R/model_get_offset.R	100.00% <100.00%> (ø)
R/model_get_response.R	100.00% <100.00%> (ø)
R/model_get_weights.R	100.00% <100.00%> (ø)
R/tidy_add_n.R	100.00% <100.00%> (ø)
R/tidy_add_reference_rows.R	100.00% <100.00%> (ø)
... and 7 more

[ddsjoberg]

Looks amazing. Love the update