Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

803 data reactivity@main #804

Closed
wants to merge 12 commits into from
Closed

Conversation

nikolas-burkoff
Copy link
Contributor

@nikolas-burkoff nikolas-burkoff commented Feb 7, 2023

Closes #803

Lots of TODOs in the code - especially the make id's unique of modules in teal::init

library(teal)

delay_module <- function(label = "delay", sleep_time = 0.5) {
  module(label = label, server = function(id, data) {
    moduleServer(id, function(input, output, session) {

      observeEvent(data[[1]](), {
        print(sprintf("trigger for module %s of %f", label, sleep_time))
        Sys.sleep(sleep_time)
        print(sprintf("trigger for module %s of %f completed", label, sleep_time))
      })

    })
  })
}


my_iris <- iris
my_iris$Species[1] <- NA

app <- init(
  data = teal_data(dataset("IRIS", my_iris )),
  modules = modules(
    delay_module("A"),
    modules(label = "Tab2", delay_module("B")),
    modules(label = "Tab3", delay_module("C"), delay_module("D")),
    delay_module("E")    
  )
)

runApp(app)

Nikolas Burkoff added 2 commits February 3, 2023 16:58
@nikolas-burkoff nikolas-burkoff marked this pull request as ready for review February 8, 2023 15:00
@gogonzo gogonzo self-requested a review February 9, 2023 12:21
@@ -177,6 +177,8 @@ init <- function(data,
modules <- do.call(teal::modules, modules)
}

# TODO make module names unique here
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

best to do this after the reporter module has been added

Copy link
Collaborator

@mhallal1 mhallal1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this branch with the exploratory app, it seems that the reactivity is not triggered there when changing the tabs of modules so none of the modules is calculated at the beginning or when changing.

@gogonzo
Copy link
Contributor

gogonzo commented Feb 9, 2023

I tested this branch with the exploratory app, it seems that the reactivity is not triggered there when changing the tabs of modules so none of the modules is calculated at the beginning or when changing.

I had the same but then I've tried individual modules and they are working. Problem found in the sample apps is that they have first tab which doesn't need data. When I removed front-page and dataviewer modules everything was fine.

@mhallal1
Copy link
Collaborator

mhallal1 commented Feb 9, 2023

I tested this branch with the exploratory app, it seems that the reactivity is not triggered there when changing the tabs of modules so none of the modules is calculated at the beginning or when changing.

I had the same but then I've tried individual modules and they are working. Problem found in the sample apps is that they have first tab which doesn't need data. When I removed front-page and dataviewer modules everything was fine.

Agreed, plus some shinyvalidate errors in regression and bivariate modules, investigating these.

@nikolas-burkoff
Copy link
Contributor Author

Also I think a few modules (PCA, outliers,...) aren't happy

@donyunardi
Copy link
Contributor

I also experienced the same thing that were mentioned.
I guess we can close this without merging in favor of #806?

@mhallal1
Copy link
Collaborator

I also experienced the same thing that were mentioned. I guess we can close this without merging in favor of #806?

If we get agreement on #806 then we will close this one.

@github-actions
Copy link
Contributor

Unit Tests Summary

    1 files    13 suites   12s ⏱️
150 tests 147 ✔️ 0 💤 2  1 🔥
294 runs  289 ✔️ 0 💤 4  1 🔥

For more details on these failures and errors, see this check.

Results for commit 1c702d1.

@nikolas-burkoff
Copy link
Contributor Author

Closed in favor of #806

@nikolas-burkoff nikolas-burkoff deleted the 803_data_reactivity@main branch February 22, 2023 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG]: teal apps are slow and vulnerable to the change in the data
4 participants