Mr 19 cov forest tab

[BalancedmonkeY]

Work involved further modularising of the base app - so may want to view changes by individual commits, or most recent three, to see more detailed changes.

[BalancedmonkeY]

All code looks good, and the things I can try all work. There are some things I can't try until they are implemented, e.g. sensitivity analysis. Please change the function names to upper camel case where they aren't already (unless this interferes with existing code).

I have changed some functions (and variable names) to follow the style guide. However, I haven't changed the Shiny module functions to upper camel case as they are a bit different. Janion hopes to chat about this on Friday.

[BalancedmonkeY]

The step in numericInput isn't updating if you do a new analysis without closing the app, so please add the step change to the updateNumericInput functions in bayesian_forest_plot_plus_stats_server in _forest_plot_plusstats.R.

The new name of gemtctau in _fnanalysis.R will cause a conflict when merging into dev, because this function is no longer in _fnanalysis.R. Might not be a problem.

I don't think the warnings in _covariate tab/forest_plotpage.R will ever be displayed, because the analysis is blocked when you select the radio button for one of the unimplemented analyses. If you want to ignore this for now I'm happy to approve, but the redundant code should be removed at some point.

Good spot on the updateNumericInput - thanks. I've also removed the warning for the covariate analysis as you're right, its not needed. For now, the feature branch doesn't have those changes, so the renaming of gemtctau should be okay for now. But I have done some rearranging to help with the eventual merge.

[JanionNevill]

The step in numericInput isn't updating if you do a new analysis without closing the app, so please add the step change to the updateNumericInput functions in bayesian_forest_plot_plus_stats_server in _forest_plot_plusstats.R.

You added steps to the min, but not the max. Was there a reason for this?

EDIT:

After discussion, it was realised that the checks should be on the outcome measure, not the meta outcome. Please changes this to reflect better when the measure is logarithmic or linear.

While you're there....

• The formatting is inconsistent with spaces around the =.
• The If statement should also have and else statement to throw an error if the outcome is not one of the supported options.
• The reactive doesn't need to be assigned to a variable x at the beginning, it can just be called in each relevant location.

[BalancedmonkeY]

The step in numericInput isn't updating if you do a new analysis without closing the app, so please add the step change to the updateNumericInput functions in bayesian_forest_plot_plus_stats_server in _forest_plot_plusstats.R.

You added steps to the min, but not the max. Was there a reason for this?

EDIT:

After discussion, it was realised that the checks should be on the outcome measure, not the meta outcome. Please changes this to reflect better when the measure is logarithmic or linear.

While you're there....

• The formatting is inconsistent with spaces around the =.
• The If statement should also have and else statement to throw an error if the outcome is not one of the supported options.
• The reactive doesn't need to be assigned to a variable x at the beginning, it can just be called in each relevant location.

Implemented all this. However, for RD, MD, and SMD, the lower limit will step up by 0.1 first, then 1 onwards. I don't really know why it's doing this, and is something the app already does...

[BalancedmonkeY]

For the box/button where you enter the covariate value, there needs to be some text to say that's what it is. Also, I'm not convinced it's in the best place. I would have put it lower down, say underneath the "choose type of regression coefficient" selection box. This might also sort out the alignment, because it's currently slightly too high. If other people prefer the current position I'm happy to leave it.

Can we reorder the options in the "choose type of regression coefficient" selection box? I would have put them in order of the number of parameters, i.e. shared, exchangeable, unrelated (or the reverse).

Regarding the covariate value box: I know that the initial reason for having this where it is, rather than within the tab with the other 'run model' options, is so that the user can see what it's currently set at across all the sub-tabs (as it affects most of them). Maybe this (and you're other point) should be discussed at the main F2F meeting on Monday? Also, as its 'off-task', I'd be grateful if this is captured elsewhere and we try to get this pull request completed as multiple tasks are waiting on it.