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

Purpose for print.report_table containing several package calls #54

Closed
humanfactors opened this issue Oct 23, 2019 · 2 comments
Closed
Labels
3 investigators ❔❓ Need to look further into this issue

Comments

@humanfactors
Copy link
Member

humanfactors commented Oct 23, 2019

Question and context

This obviously isn't a bug, but I'm trying to work out the logic behind why print.report_table requires nested calls to both insight, and then parameters. There's something I've missed along the way in terms of why format table is contained within insight if it's used for pretty printing. Indeed, this could have been discussed in another thread and I've missed it.

#' @export
print.report_table <- function(x, ...) {
  table <- insight::format_table(parameters::parameters_table(x))
  cat(table)
}

If I'm not mistaken, in effect the final call ends up being something like this within report:

cat(insight::format_table(parameters::parameters_table(report::report(a_model_object))))

I would like better understand this design decision, and the history behind it. Pending that answer, I do think it would be worthwhile documenting some of these processes more explicitly within the functions themselves (i.e., dev notes). This is just a suggestion, but it certainly makes contributing and debugging a little easier!

For context, the reason I ask this is that I have been experimenting with some Rmarkdown printing, but it's certainly not trivial to implement within the ecosystem of report. I wanted to follow how these tables were generated... here I am!

Hopefully I haven't missed something written elsewhere in the project. Cheers. 😀

@DominiqueMakowski
Copy link
Member

Very good question!

I think that insight::format_table() is mainly a table formatted converting a dataframe to a table-like display when printed in the console (adding columns separators, spaces etc.). Whereas parameters_table is mainly a function that renames some columns for pretty printing, for instance if it detects the presence of columns named CI_low and CI_high, it will merge into one column named for instance 95% CI

PS: we are currently finishing a restructuration that has seen some parts of parameters and report going into the new effectsize package. Hence, it is likely that report will have to be restructured quite a bit in the days to come.

In particular, I am thinking about creating two main functions, model_table and model_text. The former would pull performance, parameters and effect size data and merge them together nicely (rmarkdown printing will probably have to be connected at this level), and model_text will transform this table to text. Then, the report function will mainly run these two subfunctions and return it all.

@strengejacke strengejacke added help us 👀 Help is needed to implement something 3 investigators ❔❓ Need to look further into this issue and removed help us 👀 Help is needed to implement something labels Mar 19, 2020
@DominiqueMakowski
Copy link
Member

Closing as part of the house-cleaning related to the recent rewriting (so I can see clearer in the issues), but that's still an area of improvement.

Currently the printing of tables relies on insight and parameters:

report/R/report_table.R

Lines 100 to 102 in cbdd7f9

print.report_table <- function(x, ...) {
cat(insight::format_table(parameters::parameters_table(x, ...), ...))
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 investigators ❔❓ Need to look further into this issue
Projects
None yet
Development

No branches or pull requests

3 participants