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

datasummary_balance does not use siunitx #399

Closed
vincentarelbundock opened this issue Nov 2, 2021 · 7 comments
Closed

datasummary_balance does not use siunitx #399

vincentarelbundock opened this issue Nov 2, 2021 · 7 comments
Labels
bug Something isn't working

Comments

@vincentarelbundock
Copy link
Owner

Problem is that the final table is a combination of several tables produced by output="data.frame", so the rounding is done in a pre-processing stage, without siunitx.

@vincentarelbundock vincentarelbundock added the bug Something isn't working label Nov 2, 2021
@acarril
Copy link

acarril commented Mar 8, 2024

This is still an issue on modelsummary_1.4.5. Any insights as to how difficult it would be to fix? I could try to tackle this if you accept PRs.

In relation to this, the documentation for datasummary_balance mentions the necessity for the siunitx Latex package and that the default behavior is to enclose numbers in \num{}, which is not happening. Maybe it would be a good idea to remove these mentions from the documentation until this issue is fixed, or at least link to it?

By default, LaTeX tables enclose all numeric entries in the \verb{\\num\{\}} command
from the siunitx package. To prevent this behavior, or to enclose numbers
in dollar signs (for LaTeX math mode), users can call:

@vincentarelbundock
Copy link
Owner Author

Ah yeah, that would be great. I'm sure the solution only takes a few lines of code, but finding the right spot in the logic might (or might not) be tricky.

If you look in the main function, there are calls to datasummary(output="dataframe"). If you dig, you'll find that the numeric formatter disables siuntix wrapping when the output is a dataframe.

This is important, usually, but in this case it bites us.

What we need is to find a way to disable this check, and let datasummary() format with \num{} in the special case where the original calling function is datasummary_balance()

@acarril
Copy link

acarril commented Mar 8, 2024

I see, thanks for the details. I'll try to work on it this weekend and see if I can come up with a solution.

@acarril
Copy link

acarril commented Mar 8, 2024

What we need is to find a way to disable this check, and let datasummary() format with \num{} in the special case where the original calling function is datasummary_balance()

If this is an approach that you think would work, maybe it can be implemented fairly quickly using something like sys.call() to inspect the call stack. For instance, consider

foo <- function() {
  caller <- sys.call(-1)  # sys.call(-1) returns immediate caller
  return(caller)
}

bar <- function() {
  foo()
}

Here, running foo() would return NULL (i.e. no parent caller), but running bar() returns bar() (the caller):

r$> foo()
NULL

r$> bar()
bar()

I am not familiar with this codebase, but by taking a quick look, I suppose that the "numeric formatter" that you mention is in fmt_factory.R. I'd have to look more closely at how datasummary() is formatting its output. Let me know if I'm missing something!

@vincentarelbundock
Copy link
Owner Author

Hmm, maybe this would work. But there's also a package-specific settings_get()/settings_set() mechanism that we can use to pass this kind of information across function calls. So introspecting the call stack might be overkill.

Here's a start of a pull request with a simple change that seems to work. Need to check why some tests are failing and test various cases, but that's a start...

#720

@vincentarelbundock
Copy link
Owner Author

Alright, I think I’ve got this working now. It requires the development versions of both `tinytable:

remotes::install_github("vincentarelbundock/tinytable")
remotes::install_github("vincentarelbundock/modelsummary")

This ended up being one of those things that fell into a rather arcane corner of the package, so it was easier to fix myself.

That said, I really appreciate your offer to help! And I really don’t want my taking over the issue to discourage you from making contributions if you run into other issues that you might care about.

I tagged a couple issues as “good first” if you or anyone else want to try something. https://github.com/vincentarelbundock/modelsummary/issues?q=is%3Aissue+is%3Aopen+label%3A%22good+first+issue%22

No pressure, obviously!

library(modelsummary)

dat <- transform(
  mtcars,
  cyl = factor(cyl),
  gear = factor(gear)
)

datasummary_balance(~am, data = dat, stars = TRUE, output = "latex")
\begin{table}
\centering
\begin{tblr}[         %% tabularray outer open
]                     %% tabularray outer close
{                     %% tabularray inner open
colspec={Q[]Q[]Q[]Q[]Q[]Q[]Q[]Q[]},
cell{1}{3}={c=2,}{halign=c,},
cell{1}{5}={c=2,}{halign=c,},
column{1}={halign=l,},
column{2}={halign=l,},
column{3}={halign=r,},
column{4}={halign=r,},
column{5}={halign=r,},
column{6}={halign=r,},
column{7}={halign=r,},
column{8}={halign=r,},
row{1}={halign=c,},
hline{11}={1,2,3,4,5,6,7,8}{solid, 0.05em, black},
}                     %% tabularray inner close
\toprule
&  & 0 &  & 1 &  &  &  \\ \cmidrule[lr]{3-4}\cmidrule[lr]{5-6}
&    & Mean & Std. Dev. & Mean & Std. Dev. & Diff. in Means & Std. Error \\ \midrule %% TinyTableHeader
mpg  &   & \num{17.1}  & \num{3.8}   & \num{24.4}  & \num{6.2}  & \num{7.2}**     & \num{1.9}  \\
disp &   & \num{290.4} & \num{110.2} & \num{143.5} & \num{87.2} & \num{-146.8}*** & \num{35.0} \\
hp   &   & \num{160.3} & \num{53.9}  & \num{126.8} & \num{84.1} & \num{-33.4}     & \num{26.4} \\
drat &   & \num{3.3}   & \num{0.4}   & \num{4.0}   & \num{0.4}  & \num{0.8}***    & \num{0.1}  \\
wt   &   & \num{3.8}   & \num{0.8}   & \num{2.4}   & \num{0.6}  & \num{-1.4}***   & \num{0.2}  \\
qsec &   & \num{18.2}  & \num{1.8}   & \num{17.4}  & \num{1.8}  & \num{-0.8}      & \num{0.6}  \\
vs   &   & \num{0.4}   & \num{0.5}   & \num{0.5}   & \num{0.5}  & \num{0.2}       & \num{0.2}  \\
carb &   & \num{2.7}   & \num{1.1}   & \num{2.9}   & \num{2.2}  & \num{0.2}       & \num{0.7}  \\
&   & N            & Pct.         & N            & Pct.        &                  &             \\
cyl  & 4 & \num{3}     & \num{15.8}  & \num{8}     & \num{61.5} &                  &             \\
& 6 & \num{4}     & \num{21.1}  & \num{3}     & \num{23.1} &                  &             \\
& 8 & \num{12}    & \num{63.2}  & \num{2}     & \num{15.4} &                  &             \\
gear & 3 & \num{15}    & \num{78.9}  & \num{0}     & \num{0.0}  &                  &             \\
& 4 & \num{4}     & \num{21.1}  & \num{8}     & \num{61.5} &                  &             \\
& 5 & \num{0}     & \num{0.0}   & \num{5}     & \num{38.5} &                  &             \\
\bottomrule
\end{tblr}
\end{table} 

@acarril
Copy link

acarril commented Mar 9, 2024

Awesome! Thank you for the fix.

That said, I really appreciate your offer to help! And I really don’t want my taking over the issue to discourage you from making contributions if you run into other issues that you might care about.

Not discouraged at all, I appreciate it that you took the time do it. I'll keep my eyes peeled!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants