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

Add rvar_col argument to all *.data.frame() methods #666

Closed
wants to merge 4 commits into from

Conversation

mattansb
Copy link
Member

@mattansb mattansb commented Aug 22, 2024

Fixes #604

grid <- data.frame(
  A = letters[1:2],
  B = c(2, 3),
  val = posterior::rvar(array(rnorm(1200), dim = c(600, 2)))
)

# Pull rvar column:
bayestestR::p_direction(grid$val)
#> Probability of Direction
#> 
#> Parameter |     pd
#> ------------------
#> x[1]      | 53.50%
#> x[2]      | 52.67%

# Or pass the data frame and tell the function what column has rvars:
bayestestR::p_direction(grid, rvar_col = "val")
#>   A B          val        pd
#> 1 a 2 0.062 ± 0.99 0.5350000
#> 2 b 3 0.037 ± 1.00 0.5266667

# Original behavior is maintained when not specifying rvar_col:
bayestestR::p_direction(grid)
#> Probability of Direction
#> 
#> Parameter |   pd
#> ----------------
#> B         | 100%

@strengejacke
Copy link
Member

Could you also add a test?

@strengejacke
Copy link
Member

And update the rd-file, see failing tests

@mattansb
Copy link
Member Author

This is just a proof of concept. If we'd agree we want this, I'd make a much more extensive PR.

@strengejacke
Copy link
Member

@easystats/core-team WDYT? I'd say we can implement this...

strengejacke added a commit that referenced this pull request Aug 31, 2024
@strengejacke
Copy link
Member

@mattansb I don't know how to integrate this PR into my PR, so I just copied your code over to #668. We may close this PR then.

@mattansb
Copy link
Member Author

Oh, I wouldn't do that (it is very "raw")...
Maybe just leave it to me for this PR, I'll get working on it soon (it needs to be made more efficient anyway) and make all the global changes needed.

@strengejacke
Copy link
Member

strengejacke commented Aug 31, 2024

Maybe just for this particular change in p_direction()? Since I'm already working on it, and it's related to the issue from Brenton. You could extend to other functions later... I guess we will else run into conflicts when we leave this PR open, because we have different modifications made to the p_direction.data.frame() method in both our PRs.

@mattansb
Copy link
Member Author

I'm not worried about conflicts - this whole branch was just a quick and dirty proof of concept. I shouldn't have made a PR. Let's delete it.

You can integrate the changes if you want, but I will be completely overwrite those later on.

@strengejacke
Copy link
Member

Yes, that's ok!

@strengejacke strengejacke deleted the data-frame-with-rvar branch August 31, 2024 10:30
strengejacke added a commit that referenced this pull request Aug 31, 2024
* Usability improvements for pd() and pd_to_p()
Fixes #665

* docs

* add as_p

* Work on #664

* add tests

* add tests

* tests for as..vector

* news

* include #666

* news

* news

* lintr
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.

Add support for data frames with an rvar column
2 participants