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

Data/filter block standard example is no longer functional #172

Closed
nbenn opened this issue Jan 10, 2024 · 6 comments
Closed

Data/filter block standard example is no longer functional #172

nbenn opened this issue Jan 10, 2024 · 6 comments

Comments

@nbenn
Copy link
Collaborator

nbenn commented Jan 10, 2024

Using latest main I have trouble running our standard example

x <- new_stack(
  new_data_block,
  new_filter_block
)

serve_stack(x)

The resulting app is no longer functional in that the column drop down for filter choices does not work. I suspect this is due to the recent changes in block server logic @christophsax.

@christophsax
Copy link
Collaborator

christophsax commented Jan 10, 2024

Before my changes, at a985dc9, I see this:

image

My changes silence the validation (will fix this) but the underlying problem was there before. Perhaps introduced by the changes in generate_server.stack(), @DivadNojnarg?

@DivadNojnarg
Copy link
Collaborator

The filter is working well in the workspace branch (which is main from this morning including #169 without the latest PR).

@christophsax
Copy link
Collaborator

@DivadNojnarg, I have a replication problem. When I check out your branch 121-workspace-server and run Nicolas' code above, I see the same picture as in my screenshot. Do you get something different?

Perhaps the errors are meaningful and the user inputs need to be tweaked?

@DivadNojnarg
Copy link
Collaborator

DivadNojnarg commented Jan 11, 2024

@nbenn @christophsax : shouldn't it be:

stack <- new_stack(
    data_block,
    filter_block
)

serve_stack(stack)

Calling new_* does not initialises the block:

filter_block <- function(data, ...) {
  initialize_block(new_filter_block(data, ...), data)
}

and we need this to initialise fields:

initialize_block.transform_block <- function(x, data, ...) {
  env <- list(data = data)

  for (field in names(x)) {
    x[[field]] <- initialize_field(x[[field]], env)
    env <- c(env, set_names(list(value(x[[field]])), field))
  }

  x
}

EDIT: I checked the codebase 4 months ago with one the first working version: https://github.com/blockr-org/blockr/tree/54ae5802b4986165fb9fdc91a86562b727add47f. The readme example also did not contain any call to new_*_block. (devtools::install_github("blockr-org/blockr@54ae580"))

Below is when I call new_filter_block
Screenshot 2024-01-11 at 13 08 27

Below is when I call filter_block
Screenshot 2024-01-11 at 13 09 27

This isn't due to the latest PRs.

@nbenn
Copy link
Collaborator Author

nbenn commented Jan 11, 2024

@DivadNojnarg @christophsax I was not precise when stating that this worked previously. My comparison was against the fix I had proposed in #161. With that I get for a stack initialized with the basic block constructors something that works:

With branch origin/159-update-loop and code

stack1 <- new_stack(
  new_data_block,
  new_filter_block
)

serve_stack(stack1)
Screen.Recording.2024-01-11.at.16.22.48.mov

Two things:

  1. I should not have used the basic constructors but rather the "user facing" variants (without new_ prefixes).
  2. The assertion that the new server logic is identical w.r.t. non-server fields is incorrect. This is not meant as criticism and it is not necessarily a problem, I'm just stating a fact.

The problem is caused in

https://github.com/blockr-org/blockr/blob/008965264e9843ff43defe4692434d81decc46e4/R/field.R#L398-L400

a change introduced with ad4b55c. The way this change was introduced has the following issue:

I will open a separate issue.

I leave it up to you @christophsax if you want to investigate why my implementation would "self-heal" a not properly initialized field while yours does not. It might have to do with your choice of ignoreInit in

https://github.com/blockr-org/blockr/blob/008965264e9843ff43defe4692434d81decc46e4/R/server.R#L132

It is unclear to me whether we want ignoreInit here to be TRUE or FALSE. For #161 I went with default (FALSE) b/c I did not understand the reason for not starting with an update iteration right away. There might be a good reason though, I just can't remember properly.

@DivadNojnarg
Copy link
Collaborator

Closing as 0.0.2 is functional

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants