bayestestR broken after revising pd().

[strengejacke]

This PR wasn't ready, and breaks parts of the code, e.g.

library(bayestestR)
x <- distribution_normal(4000)

describe_posterior(
  x,
  centrality = "all",
  dispersion = TRUE,
  test = "all",
  ci = 0.89,
  verbose = FALSE
)
#> Error in fix.by(by.y, y): 'by' must specify a uniquely valid column

^{Created on 2023-07-31 with reprex v2.0.2}

Because those functions expect numerics instead of data frames for pd(), I guess.

Originally posted by @strengejacke in #618 (comment)

[DominiqueMakowski]

@mattansb would adding as.numeric() be sufficient here?

[mattansb]

I have no idea how describe posterior does all the merging. I think pd and p-map already have a as.numeric method.

[DominiqueMakowski]

I'm on it but it's a weird error

[DominiqueMakowski]

describe_posterior(x, test = "p_map")

Error in fix.by(by.y, y) : 'by' must specify a uniquely valid column
7.
stop(ngettext(sum(bad), "'by' must specify a uniquely valid column",
"'by' must specify uniquely valid columns"), domain = NA)
6.
fix.by(by.y, y)
5.
merge.data.frame(out, test_pmap, by = merge_by, all = TRUE)
4.
merge(out, test_pmap, by = merge_by, all = TRUE) at describe_posterior.R#474
3.
.describe_posterior(posteriors, centrality = centrality, dispersion = dispersion,
ci = ci, ci_method = ci_method, test = test, rope_range = rope_range,
rope_ci = rope_ci, keep_iterations = keep_iterations, bf_prior = bf_prior,
BF = BF, ...) at describe_posterior.R#540
2.
describe_posterior.double(x, verbose = FALSE, test = "p_map") at describe_posterior.R#109
1.
describe_posterior(x, verbose = FALSE, test = "p_map")

it also fails with other tests 🤔 🤔

[DominiqueMakowski]

It happens here, we're trying to merge these dfs by the cols but there is no Parameter columns... is it that last PR that removed it??

(disregard the print())

[strengejacke]

This code is no longer run, because p_map() now always returns a data frame:

bayestestR/R/describe_posterior.R

Lines 243 to 249 in dc049fd

	if (!is.data.frame(test_pmap)) {
	test_pmap <- data.frame(
	Parameter = "Posterior",
	p_map = test_pmap,
	stringsAsFactors = FALSE
	)
	}

Since the structure of the returned data frame for numeric input in p_map() differs from those in models, it now fails.

[DominiqueMakowski]

Do you think we should re-add a Parameter argument to the output of p_dir and p_map or fix it at the merging level?

I should have checked the tests more thoroughly 🤕 (and not listen to Mattan's mellow voice softly whispering merge it in my ear)

[strengejacke]

I think the easiest thing would be that the numeric method returns an additional column named Posterior, like what we do in describe_posterior(), i.e. something like

   data.frame( 
     Parameter = "Posterior", 
     p_map = pmap, 
     stringsAsFactors = FALSE 
   ) 

[mattansb]

Okay, I can fix that.

[mattansb]

Okay, I've changed the .numeric() method for p_map(), p_direction(), map_estimate() and p_significance(), and describe_posterior() now works:

describe_posterior(x)
#> Warning: Could not estimate a good default ROPE range. Using 'c(-0.1, 0.1)'.Summary of Posterior Distribution
#> 
#> Parameter | Median |        95% CI |     pd |          ROPE | % in ROPE
#> -----------------------------------------------------------------------
#> Posterior |   1.04 | [-0.91, 2.92] | 86.00% | [-0.10, 0.10] |     5.89%

However, this did change the printing method -

Before:

library(bayestestR)

x <- rnorm(1000, 1)
pd(x)
#> Probability of Direction: 86.00%

After:

pd(x)
#> Probability of Direction
#> 
#> Parameter |     pd
#> ------------------
#> Posterior | 86.00%

I don't know if you want to preserve the previous behavior or not...

[strengejacke]

I'd say the print is OK.

[DominiqueMakowski]

That's okay

[strengejacke]

Can this be closed?