Add pooling argument to analyse_mi_data() for unknown imputation methods

[bailliem]

Closes #50

Summary

analyse_mi_data() can now analyse an already-imputed (ADMI) dataset when the imputation method is unknown, via a new pooling argument — addressing the scenario raised in #50 where the method object is unavailable.

• New pooling argument ("rubin", "bootstrap", "jackknife", "bmlmi"): supply either method (unchanged behaviour, fully backward compatible) or pooling directly. Both together must agree; neither errors informatively.
• New exported helper get_pooling(method) — the single source of truth mapping rbmi method objects to pooling strategies (as proposed in analyse_mi_data() when the exact imputation method is unknown. #50).
• When only pooling is supplied, the data defines the number of imputations (no n_samples check) and an internal stand-in method object sized to the data is constructed so rbmi::pool() validation passes. print()/summary() label these objects <not supplied; pooling = "rubin"> rather than claiming a method that was never given.
• pooling = "rubin" is the documented recommendation for data of unknown provenance; "bmlmi" alone errors (D not inferable from data); "bootstrap" with < 2 imputations errors rather than silently pooling to NAs.
• Design spec and implementation plan included under docs/superpowers/; also fixes pre-existing roxygen unresolved-link warnings for @noRd internals in ard_conversion.R.

The pooling/get_pooling() design is deliberately shaped to be liftable into rbmi core per openpharma/rbmi#589.

Test plan

• 95 tests in test-analyse_mi_data.R (34 new): validation error paths, method/pooling conflict, path-equivalence (pooling-only vs method produce identical pooled results), end-to-end rbmi::pool() for rubin/bootstrap/jackknife stand-ins, provenance labelling, guards.
• 7 new tests in test-get_pooling.R covering all four method classes and error paths.
• Full suite: 1075 passing; 2 pre-existing test-utils.R failures are a local rstan/Rcpp toolchain issue unrelated to this change.

[wwojciech]

Thank you @bailliem

Overall, this looks good. One design comment, though: I'm not convinced it's a good idea to have method and pooling as separate arguments when they are effectively 100% dependent on each other. Having two parameters representing the same concept feels like unnecessary overhead, and it complicates the API.

If backward compatibility is a must, I think it would be cleaner to keep a single method argument. It could accept either:

1. an rbmi method object (the current behaviour), or
2. a character string specifying the pooling strategy (e.g. "rubin").

If backward compatibility is not a requirement, I would simply keep a single method argument that accepts the name of the pooling strategy as a character string.

[luwidmer]

I tend to agree with @wwojciech about the argument (unless we are planning to deprecate and then in a subsequent version remove the old one).

In addition, is there some way to prevent use of e.g., pooling="rubin" on data that was generated by pooling="bootstrap"? Seems to me like there's potential to introduce silent/subtle errors there?

[bailliem]

Thanks @wwojciech and @luwidmer , agreed on both issues. One thing that helps; there's nothing to be backward compatible with. pooling and get_pooling() only exist on this branch and neither has been near CRAN, so no deprecation cycle from my perspective. I'd propose to drop pooling and let method take a method object or a string, which aligns us with the current CRAN release:

analyse_mi_data(ADMI, vars, method = method_bayes(n_samples = 100))  # unchanged
analyse_mi_data(admi_csv, vars, method = "rubin")                    # new

@luwidmer , on your second point, {rbmi} never asks to name a pooling strategy, this is inferred inside the object from draws() to impute() to analyse() (I think!). This function takes a plain data frame, which is a bit of a hack and not robust to errors i.e. impute_obj$method already has the real method and get_imputed_data() throws it away, so I'd rather keep it on the ADMI as an attribute and check it at analysis time.

Three things I'd rather decide before wrapping this up:

1. Is a string on method the right spelling? It's @wwojciech's suggestion and I'm happy with it, but if {rbmi} would want it different I'd rather match now. I was going to mark it experimental so we can follow whatever gets decided.
2. How strict should the provenance check be? My instinct is a hard error when the stamp and the supplied method disagree, with attr(data, "rbmiUtils_method") <- NULL as the deliberate override, rather than a warning or yet another argument.
3. Is an interim fix here even worth it? @wwojciech , I've come round to your view in Feature request: Add analyse_mi_data(). rbmi#589. The stand-in method object only exists to satisfy validate.analysis(), so a good chunk of this PR is scaffolding that disappears if the function lives in {rbmi}. @luwidmer what do you think?

Either way I'll fix a bug I found: we prepend "rbmiUtils_standin" to the method class, which shifts class(method)[[2]] from "bayes" to "method", and rbmi:::as_analysis() switches on exactly that. Its not a big issue because rbmiUtils uses its own constructor, but it would the moment this moves upstream. I'll append instead.

@tobiasmuetze FYI.