tdaverse / ggtda

ggplot2 extension to visualize persistent homology
https://tdaverse.github.io/ggtda/
GNU General Public License v3.0
19 stars 5 forks source link

Question: shiny module support warranted? #6

Closed peekxc closed 3 years ago

peekxc commented 4 years ago

Shiny modules seem to provide the most supported solution of encapsulating blocks of R code geared for building shiny apps quicker (another example article).

I was wondering if this package would be interested in incorporating some degree of modularization?

As an example, I forked this repo and added a few example shiny modules. Collectively, they create a set of UI buttons for choosing between persistence data plot types. There's also a buttons for doing things like adding the frontier, filtering by some noise threshold, choosing between H0, H1, etc., automatic reactive callbacks for detecting the nearest feature a user clicked, etc.

An example shiny app, also shown below, demonstrates the simplification modularization enables:

library(shiny)
library(ggtda)
library(TDAstats)

## Example from TDAstats calculate_homology function help page
library(TDAstats)
data("circle2d") # unit circle (Betti-1 number = 1)
circ.phom <- as.data.frame(calculate_homology(circle2d))
circ.phom$dimension <- as.factor(circ.phom$dimension)

## assume persistence data is named 'dgm' 
dgm <- circ.phom

ui <- fluidPage(
  titlePanel("Persistence shiny plot demo"), 
  sidebarLayout(
    sidebarPanel(
      dmg_select_ui(id = "select_pers"), ## selects the type of persistence plot to show
      dgm_var_UI(id = "var_pers")        ## parameters to tweak like e.g. noise threshold 
    ), 
    mainPanel(
      linkedPersistenceOutput("pers_plot"), 
      verbatimTextOutput("text_out")
    )
  )
)

server <- function(input, output, session){
  ## Get the type of plot to interact with  
  dgm_selection <- callModule(dgm_select, "select_pers")

  ## Get the selected parameters associated with the UI elements
  dgm_vars <- callModule(dgm_var, "var_pers", dgm)

  ## Create the persistence plot
  res <- callModule(linkedPersistence, "pers_plot", dgm, dgm_selection, dgm_vars)

  ## Show the index of the selected feature
  output$text_out <- renderText({ res$selected_feature_idx() })
}
shinyApp(ui, server)

The modules aren't entirely meant for the package as-is. I'm sure there's lots of different ways to improve them, just wondering the scope of this package, since I plan to do this otherwise with some of my other packages.

corybrunson commented 4 years ago

I've not paid much attention to Shiny recently, so this idea is new and exciting to me. My own attitude toward extensions (most of the packages i've written) is that they should be "one-dimensional", i.e. they should extend the original package in one key respect. But i don't know whether and how Shiny modules are typically used, in extensions or in general. Is there something i could read to get me up to speed?

Regardless, i think this is definitely something worth making available in a convenient way, being naturally appropriate to persistence analysis. (Thanks for putting together the example!) @rrrlw what's your take on this?

rrrlw commented 4 years ago

Just ran the example and I love it! To me, the Shiny window has a lot of educational value for people new to TDA and could also serve as a useful sandbox for experts. Like @corybrunson, my primary concern is whether ggtda (as an extension of ggplot2) has a sufficiently broad scope to include shiny apps. However, even if it's strictly not supposed to, I am willing to include Shiny functionality as it does fall within the scope of "visualizing persistent homology". My preliminary thoughts are as follows:

However, my knowledge of Shiny is very limited. @peekxc: does the above make sense or is my comprehension of Shiny faulty (particularly with regards to the Shiny additions to ggtda)? Also, would you be willing to contribute to ggtda and/or TDAstats?

And @corybrunson: I would only be willing to add this to ggtda with your blessing. I agree that this functionality should be included somewhere, whether in ggtda, an existing package, or a new package. What do you think of including some Shiny functionality in ggtda?

Would also love to hear both your thoughts on including the super user-friendly Shiny function in TDAstats. On a tangential note, I've been meaning to use gganimate to include some animations of barcodes, persistence diagrams with corresponding Vietoris-Rips complexes (showing how circles in V-R expand and create edges in V-R & change barcode/persistence diagram's features) in either TDAstats or a new package so if we decide against including Shiny stuff in ggtda, it could pair well w/ animations.

P.S. - very minor note, the dotted diagonal line is showing up for the "Flat" option instead of the "Diagonal" option (but I think the points are in the right places for each)

corybrunson commented 4 years ago

@rrrlw i think TDAstats is a natural place to incorporate Shiny modules, either in their usual form (whatever that is) or as user-friendly convenience functions.

That said, i'm warming to the idea of incorporating them into ggtda, in more "raw" form, at least in the absence of a general standard. They're certainly handy, and it doesn't make sense to put modules for different extensions into a single package or to make separate meta-extensions for them. Like autoplot(), any given ggplot2 extension could include them or not.

Like @rrrlw i noticed some details that i'd want to adjust in @peekxc's implementation and vignette, but that could be done quickly, and i'd be glad either to join work on the fork or just wait for a PR. : )

peekxc commented 4 years ago

But i don't know whether and how Shiny modules are typically used, in extensions or in general. Is there something i could read to get me up to speed?

You've probably already found these, but the primary resources I've been using include just the couple of pages here, here, and especially here.

I've not paid much attention to Shiny recently, so this idea is new and exciting to me. My own attitude toward extensions (most of the packages i've written) is that they should be "one-dimensional", i.e. they should extend the original package in one key respect.

I've actually had some issues trying to get the right abstraction of modularization. For example, for this package, maybe one might interpret this threshold module as 'good' design because of the single-responsibility principle:

## Noise threshold UI module 
threshold_UI <- function(id) {
  ns <- NS(id)
  numericInput(ns("noise_threshold"), label = "Noise threshold", value = 0, min = 0),
}

## Noise threshold server module
threshold <- function(input, output, session, dgm){
  updateNumericInput(session, inputId = "noise_threshold", step = min(dgm$death - dgm$birth))
  reactive({ input$noise_threshold })
}

I understand the other benefit of modularization comes with the namespacing, but to me this doesn't really achieve any simplification and therefore it doesn't seem like the right abstraction level.

However, my knowledge of Shiny is very limited. @peekxc: does the above make sense or is my comprehension of Shiny faulty (particularly with regards to the Shiny additions to ggtda)?

I think so ! Shiny modules encapsulate fixed predefined reactive behavior, so as you say maybe it's most appropriate to include the modules in ggtda and a full-fledged illustrative shiny app using said components in TDAstats.

Also, would you be willing to contribute to ggtda and/or TDAstats?

Yep!

Edit: in response to @corybrunson comment above. I'm happy to work off whatever fork / branch / repo is most convenient

rrrlw commented 4 years ago

Let's continue on the fork for ggtda and then merge w/ PR? Same for TDAstats?

Happy to add you as a collaborator w/ write access to these repos if that's easier!

rrrlw commented 4 years ago

On second thought, added you as an author in DESCRIPTION and gave you push access to ggtda - should make things easier. (Apologies @corybrunson! Just realized I should've checked with you first)

corybrunson commented 4 years ago

@rrrlw no problem! I'd only suggest to add someone as an author in the first branch (or merge) they contribute, for ease of later reference. And i'm fine with your proposed workflow. I can imagine several Shiny apps that could be build up from various different modules in this package, so if @peekxc is OK with it, then i'd like the opportunity to write some on the fork, following their examples, for practice.

Also, @rrrlw, is your idea to build modules in the more standard form here, then import them into shortcut functions in TDAstats? That sounds reasonable to me, just want to be sure we're on the same page.

Do y'all feel that this is something worth encouraging other ggplot2 extensions, other packages with strong visual components, to include? Maybe we can assess our feelings once we've done it, but if the answer turns out to be yes then it might be worth taking some sort of step to advocate for it—both for doing it and for following (or at least considering) whatever organizational scheme we find works best.

Thanks a lot @peekxc for raising the topic!

rrrlw commented 4 years ago

Yes, the author addition method you suggest is far better - I will keep it in mind for next time! Thank you for clarifying regarding the Shiny modules, what you wrote was essentially what I was thinking. I believe the original plan was that, once ggtda was on CRAN, we would use ggtda geoms (+ themes, axis labels, etc. from ggplot2) in TDAstats rather than the point or segment geoms. Analogously, I was thinking we could import Shiny modules from ggtda (once on CRAN) and add a few bells & whistles that make for a reasonable default Shiny window for TDA users (maybe multiple designs). And definitely open to revising this as we move forward!

I don't have enough experience with ggplot2 extensions to have good intuition regarding your last question. However, I imagine that ggplot2 extensions would benefit from having a Shiny module with a sample dataset that allows users to explore how a particular visualization of the sample data (or data of their choice) changes as the parameters are adjusted. I absolutely agree with your thoughts on assessing as we move forward, advocating for it (via GitHub Gists, blog posts, twitter, etc.), and seeing what the R community thinks. Excited to work on this w/ y'all!

rrrlw commented 3 years ago

See #10. Closed and moved to this repo.