Exported internal helper functions

[munoztd0]

junco uses a few non exported symbols from rbmi. To use a public, stable API and avoid relying on unexported internal symbols, can we consider exporting them in the next release?

I have added a test for the ones that did not have one.

[munoztd0]

see -> johnsonandjohnson/junco#315 @danielinteractive

[luwidmer]

@munoztd0 could you elaborate a bit in which context these functions are used in junco and why?
There's always a bit of a tradeoff between exposing a clear (and complete enough) interface and its stability when internals are changed (and reverse dependencies potentially affected). @gravesti FYI

Given this, do you have any thoughts or feedback from rbmiUtils, @bailliem & @tobiasmuetze?

[danielinteractive]

Hi @luwidmer , I can comment a bit on this topic because I adapted a few of the rbmi functions into junco. The main reasons were:

• at the time the latest rbmi released package was not available internally and therefore some code needed to be "imported" in the junco package (e.g. parallel analyse() capability, see https://github.com/johnsonandjohnson/junco/blob/main/R/rbmi.R#L402)
• some additions needed to be made and it was not clear at the time whether this would be ok for rbmi general package (e.g. variance pooling, see https://github.com/johnsonandjohnson/junco/blob/main/R/pool_rbmi.R)

I think an alternative to exporting these functions (which is clearly easiest short term, but maybe not long term) would be to revisit the forked code in junco and then integrate those into rbmi.

[gowerc]

I must admit exporting some of these makes me a little nervous as they are only really tested narrowly for how they are used in rbmi and were never intended to be more broadly used (lsmeans in particular is very narrowly tested). A good example to me is parametric_ci which I think has some very unintuitive behavior which I had a todo item to refactor at some point ( #512 ). Thats not to say no to this but it does make me a little hesitant, I will defer to @luwidmer's judgement on this.

[luwidmer]

My preference would be on a longer-term / sustainable solution. @bailliem @gowerc @gravesti @tobiasmuetze any thoughts from your end?

Also, given that we're planning to document the return values for the next version, at the very least, this should be added to the exports, too.

[tobiasmuetze]

I agree that a long-term solution would be preferable with a focus on what fits into the rbmi workflow.

[munoztd0]

So would you be ok if I had more individual and edge cases test for each of the functions and do individual PRs for each ? @luwidmer @gowerc @tobiasmuetze

[danielinteractive]

And as mentioned above, I can also imagine very well that we could summarize the updates we would need in rbmi to the existing functions, such that we would then not need to export internal helpers any longer from it.