metrumresearchgroup / TFLGenerator

The TFL generator meta repository. This includes the GUI and TFL R packages as submodules and manages the shiny application deliverable.
0 stars 0 forks source link

Architectural/vision changes #3

Open dpolhamus opened 6 years ago

dpolhamus commented 6 years ago

Since creation of the app, shinydev has released a renderUI that seems like it should be used in most of the situations we were using insertUI. While I wouldn't expect to see much of a change as a user due to this, I suspect it'll solve some of the reactivity issues we have in the app and bring the app up to speed to be a more modern implementation of dynamic UI with shiny.

The estimate is fairly high for this as we use insertUI heavily throughout the app. For example, every time a new TFL is requested, a new UI is "inserted".

https://shiny.rstudio.com/articles/dynamic-ui.html

yonicd commented 5 years ago

One option is to create a shiny module that would control layers in a plot, making them independent environments.

While this is a big investment in development it would create a more stable mechanism that the figures would be built on, making app more stable and easier to maintain for any future changes too.

The modules would be used in conjunction with insertUI.

This is also keeping with the intended use that is in the newer shiny workflows, thus moving to this framework would make keeping agile to future shiny updates more feasible.

dpolhamus commented 5 years ago

I think is a pretty important thing to look at, but it is a pretty big time investment. I think we'd need at least 60 hours to get this stable.

dpolhamus commented 5 years ago

Thinking about this more, the app is old enough at this point that I think a major overhaul of the front-end is probably warranted.

  1. Highest priority would be transitioning to using the queuing system Devin has put together.

The way this has been explained to me makes it sounds like it wouldn't be a major effort to update this. The queueing system basically dumps out an rda and builds a standalone Rscript to be run "somewhere". Major benefits of this are: 1) debugging is much simpler as you have the rda and Rscript, 2) the app itself is protected against crashes within the function applications, and 3) as each function call is executed in its own session, reactivity-hell can be mitigated.

  1. I feel like the whole concept of this app needs an update.

My preference would be to completely remove any scripting from this and force users to do DA and scripting outside of the application. The app would then load a derived dataset, and then allow users to build out a set of graphics and tables from an arbitrary package. I.e., the TFL package here, which is the collection of functions that perform PK analyses, could be swapped out with updated package versions, or packages with entirely different purposes (e.g., survival analysis / TTE type TFLs). This is a big undertaking, but would have a huge payoff in that clients could run entirely outside of the TFL generator, debug much more easily, and develop their own backend packages (possibly with help and advice from MetrumRG). As a first pass, it could just be expected to work on the existing TFL package (which should be scrubbed a little to behave better in a naked R environment, without a shiny front-end).

  1. The app uses first a totally first gen UI (it's 3 years old now) and is frankly kinda ugly for the sake of functionality at this point. An aesthetic overhaul would be welcome here.

In conclusion, this is obviously a much bigger undertaking than 60 hrs, and is likely more along the lines of a month of a dev. Again though, once something like this is complete then more enhancements along the lines of functionality could be handled on the client end with development of packages that work in and outside of the shiny frontend.