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

data_summary() works with bayestestR::ci() #483

Merged
merged 27 commits into from
Nov 27, 2024
Merged

Conversation

strengejacke
Copy link
Member

@strengejacke strengejacke commented Mar 3, 2024

library(datawizard)
data_summary(mtcars, ci = bayestestR::ci(mpg), by = c("am", "gear"))
#> am | gear |         95% CI
#> --------------------------
#>  0 |    3 | [10.40, 21.46]
#>  0 |    4 | [17.91, 24.28]
#>  1 |    4 | [21.00, 33.64]
#>  1 |    5 | [15.08, 29.96]

Created on 2024-03-03 with reprex v2.1.0

Copy link

codecov bot commented Mar 3, 2024

Codecov Report

Attention: Patch coverage is 50.00000% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 90.60%. Comparing base (26ca506) to head (4e0feb5).

Files Patch % Lines
R/data_summary.R 50.00% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #483      +/-   ##
==========================================
- Coverage   90.66%   90.60%   -0.06%     
==========================================
  Files          74       74              
  Lines        5765     5771       +6     
==========================================
+ Hits         5227     5229       +2     
- Misses        538      542       +4     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@etiennebacher etiennebacher mentioned this pull request Mar 22, 2024
@strengejacke
Copy link
Member Author

@easystats/core-team Do we want to merge this? @DominiqueMakowski ?
It's a special case, we may extend this to other functions in the future if necessary, but for now, it might be enough? I think Dom asked for this feature somewhere...

@IndrajeetPatil
Copy link
Member

I have no strong opinion either way.

Copy link
Member

@etiennebacher etiennebacher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if you want to wait for other feedback from the team but this is fine for me, I just have a couple of minor comments.

R/data_summary.R Show resolved Hide resolved
R/data_summary.R Show resolved Hide resolved
@strengejacke
Copy link
Member Author

Haha, meanwhile we have added this code:

  # check for correct length of output - must be a single value!
  if (any(lengths(out) != 1)) {
    insight::format_error(
      paste0(
        "Each expression must return a single value. Following expression",
        ifelse(sum(lengths(out) != 1) > 1, "s", " "),
        " returned more than one value: ",
        text_concatenate(vapply(dots[lengths(out) != 1], insight::safe_deparse, character(1)), enclose = "\"")
      )
    )
  }

and this PR doesn't work anymore. Let me try to fix this.

@strengejacke strengejacke merged commit 5463c93 into main Nov 27, 2024
22 checks passed
@strengejacke strengejacke deleted the data_summary_ci branch November 27, 2024 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants