WIP: Object instantiation/initialization/validation

[nbenn]

Personally I feel we are still lacking clarity in this area, so I'd like to use this space to collect some thoughts.

It's important to note that some of the things that feel like requirements might simply result from design flaws that'd be better addressed in other ways. If you feel this way about something, please make this clear.

The way I see it currently, we might have up to three things that might have to happen until an object is "fully usable":

1. object instantiation/creation
2. object initialization
3. object validation

If for example a stack is created as

stack <- new_stack(
  data_block,
  filter_block
)

The involved fields and blocks can be created, but they are not yet initialized, as we don't have field values yet. Such "uninitialized" blocks should probably not be validated yet, as it does not much sense to do so. As soon as all required inputs for a field have been provided, the field can be validated and as soon as all fields for a block are "validatable", the block can be considered initialized and the validation result is something like all(vapply(<blocks>, is_valid, logical(1L)).

We have generics

• is_initialized() currently for block and field
• validate_field() for many different types of fields

and functions

• validate_inputs()
• validate_block()

which communicate validation results to the user.

Currently (@DivadNojnarg please correct me if I'm wrong), the functions responsible for communicating validation status to the user by looking at shiny input values¹. Some issues I see with this (much of this might be a lack of proper understanding on my side):

• We now have duplicate "validity tracking" and I feel validate_inputs() is doing too much or the wrong thing here. Each field should be able to report on its validity (or initialization) status (and block validity is an aggregation of that). And separately from that, there is the issue of communicating the result of this validation to the user.
• Something like which(inputs_to_validate %in% c("expression", "submit")) is a (solvable) problem. How these fields are named is arbitrary and entirely up to the block implementer. We cannot hard-code such validation behavior to field names. If certain fields are to be excluded from this, it should be based on field type or some field attribute validatable or so.
• With a condition like length(val) == 0 || (length(val) > 0 && all(nchar(val)) == 0) you fix a global criterion for what is a valid field value. I don't think this scales well. It might very well be that "" in some field is a valid value (e.g. if a filter expression col == "" is represented as separate values col and val, val would have to be settable to "", which in this way would globally be considered an "invalid input".

Footnotes

1. @christophsax such things are important to keep an eye on if we want to provide an abstraction for how to look up user inputs (e.g. when having a module based field). ↩

[DivadNojnarg]

For the validation communication, we could call validate_inputs inside validate_block (like here) and only call validate_block within the corresponding observer.

Validation happens on the Shiny side. It is fairly limited but it's ok for now, the most important is to check whether inputs have a value and prevent from evaluating invalid expressions. The goal was also to handle the case where we change upstream data and a column does not exist anymore: user should be notified about this. I don't store the valid status within the block structure. Maybe this is a missing piece and we could have a preliminary validation running before the shiny layer.

The validate_field_* methods ensure the value stays in within the allowed bounds (for a range, numeric input, ...) and reset to factory settings if not. They should actually be renamed to correct_field or something like that (I would avoid restore, which could be misleading if we think about the serialisation and stack restoration).

[nbenn]

@DivadNojnarg sorry, I marked the discussion as "WIP", as I was still working on it, and did not want to loose what I had so far. Now it's in a (slightly) better state.

[DivadNojnarg]

Considerations for the validation.

Validation should happen at initialisation so inside validate_field makes sense because we do this:

initialize_field.field <- function(x, env = list()) {
  validate_field(
    eval_set_field_value(x, env)
  )
}

Validation should refresh in real time (as long as the app is alive), which is currently the case (update_field runs each time a field has changed):

update_field.field <- function(x, new, env = list()) {
  x <- eval_set_field_value(x, env)

  value(x) <- new

  validate_field(x)
}

When the app is running:

• validation should not crash the app.
• if FALSE, it should prevent to evaluate the block and forward the data to the downstream block.
• validation should signal to the end user when something is wrong.

Question:

• Where do we store the valid status? Within a field attribute?

Example:
For the head block, there is only one user input, a numeric field:

new_numeric_field(n_rows, n_rows_min, nrow(data))

Min and max are defined by the block developer. While min if fixed by the block developer, max depends on the input data rows. That said, the condition nrow(data) > 0 should be ensured by the previous block validation being a data block or transform block. Do we also have to validate min?

[DivadNojnarg]

Some extra thoughts discussed with @JohnCoene and @nbenn:

• There's no need to store the validation state in an attribute as this may cause serialisation issues. It is also cheap to perform and could be recalculated at run time.
• We may have different sense of what is valid or not: NA could be invalid, "" invalid, ... we need a way to allow people (including us) to customise the validity rules.
• We should split the core validation and the communication to the interface/end user. Currently this is done by the same function.
• How do we display the validation status within the stack? For instance, we have 4 blocks, assuming block 2 becomes invalid as a result of a user choice, what do we show in the interface? Should we collapse dowstream blocks (3 and 4) and show the invalid block? Second example, assuming we change the data in the first block, which makes the state of the dowstream block invalid (wrong column names), how do we reset the first block? Should empty the fields and let the user re-fill with valid choices (longer) or should we assume a reasonable default reset choice and propagate this through the entire stack (no need for user action but could be expansive and not what the user wants). I'd vote for 1.
• How do we scale to multiple stacks? Say, a block of stack1 is invalid and stack 1 is needed by stack2. How do we handle this UI side?

TBC

[DivadNojnarg]

@nbenn: I started to write down some ideas we discussed on how the core could look like in the future (WIP)

graph TD;
    
    block1[Block1]
    block2[Block2]
    block3[Block3]
    init_block[Initialize blocks]

    block1 --> |construct block| init_block
    block2 --> init_block
    block3 --> init_block

    init_block --> |set field values| evaluate_block
    
    subgraph stack
        evaluate_block[Evaluate blocks]

        evaluate_block --> |data| validation
        
        validation --> valid_block[Valid block]
        validation --> invalid_block[Invalid block]


        valid_block --> result[Stack result]
    end
    config[Validation config] --> |Custom acceptance| validation[Validation]

    stack ---> |Render blocks| shiny
    invalid_block --> |Notifications| ui

    subgraph shiny
        ui[User interface]
    end

    user[End user] --> |Input change| ui
    ui ---> |real time update| evaluate_block

Loading

From this flowchart:

• We would like to be able to instantiate the blocks independently of the stack so instead of doing new_stack(data_block, select_block), we can do (see wip: allow to pass in block calls in new_stack #350):

custom_data_block <- data_block(selected = "iris")
custom_select_block <- select_block(
  columns = "Species",
  fancy_param1 = TRUE,
  fancy_param2 = FALSE,
  submit = TRUE
)

Currently, the code base prevents us to do so:

new_select_block <- function(data, columns = colnames(data)[1], ...) {
  all_cols <- function(data) colnames(data)
 # hidden code
 ...
}

Instead we could have a default for data:

new_select_block <- function(data = data.frame(), columns = character(), ...) {
  all_cols <- function(data) colnames(data)

 fields <- list(
    columns = new_select_field(columns, all_cols, multiple = TRUE, title = "Columns")
 )
 # hidden code
 ...
}

• We would then evaluate the blocks within the stack (as we currently do) with some validation logic that checks for instance if the block field values are compatible with any input data, whether the evaluation raises an error, ... We likely want to make the validation customisable.
• The stack is rendered in the UI with relevant feedback from the validation.
• Any real time change triggered as result of the user interaction would cause relevant parts of the stack to be re-evaluated and update the status in real time.

TBC

[DivadNojnarg]

Some ideas

# We drop validate_field from here as we just want the value, whatever it is
initialize_field.field <- function(x, env = list()) {
  eval_set_field_value(x, env)
}

# Better default parameters so we can initialise blocks outside a stack
select_block <- function(data = data.frame(), columns = character(ncol(data)), ...) {

  all_cols <- function(data) colnames(data)
  fields <- list(
    columns = new_select_field(columns, all_cols, multiple = TRUE, title = "Columns")
  )

  new_block(
    fields = fields,
    expr = quote(
      dplyr::select(.(columns))
    ),
    ...,
    class = c("select_block", "transform_block")
  )
}

# validate generic for blocks
validate_block <- function(x, ...) {
  UseMethod("validate_block")
}

# Validate functions return booleans
validate_block.block <- function(x, ...) {
  res <- lgl_ply(names(x), \(field) validate_field(x[[field]]))
  if (sum(res) == length(names(x))) TRUE else FALSE
}

# We probably want to validate a field with the incoming data from
# the previous block to check for column names, values, ...
validate_field.select_field <- function(x) {
  val <- value(x)
  # TBD: to account for basic data check
  if (...) {
    FALSE
  } else {
    TRUE
  }
}

# Convenience function to loop over blocks
validate_stack <- function(blks) {
  lapply(blks, validate_block)
  ...
 TBC
}

block1 <- data_block(selected = "iris")
block2 <- select_block(data = iris, columns = "Species")

new_stack <- function(..., title = "Stack", name = rand_names()) {
  stopifnot(is_string(title), is_string(name))
  blks <- list(...)
  validate_stack(blks)
  ...
  TBC
}

[DivadNojnarg]

I still find it weird to be able to do:

block1 <- data_block(selected = "iris")
block2 <- select_block(data = iris, columns = "Species")

since it assumes that block2 must take data from block1. Who would want to create a second block with wrong data?

[DivadNojnarg]

A proposition on how to evaluate blocks. Any evaluation error is forwarded to the UI.

graph TD;
    subgraph stack
        subgraph blk1
            block1[Block 1] --> |check fields| validate_block1[Validation] --> validate_block1_ok[OK]
            validate_block1 --> validate_block1_fail[Fail]
            validate_block1_ok --> |evaluate| try_eval[Try] --> eval_block1_ok[OK] --> data_block1[Data]
            eval_block1_ok[Success]
            try_eval --> eval_block1_fail[Fail]
        end
        data_block1 -.-> block2
        subgraph blk2
        block2[Block2] --> |check fields| validate_block2[Validation] --> validate_block2_ok[OK]
        
        validate_block2 --> validate_block2_fail[Fail]
        validate_block2_ok --> |evaluate| try_eval2[Try] --> eval_block2_ok[OK] --> data_block2[Data]
        eval_block2_ok[Success]
        try_eval2 --> eval_block2_fail[Fail]
        end
        data_block2 -.-> stack_result[Stack result]
    end
    eval_block1_fail -.-x |notification| gui
    eval_block2_fail -.-x |notification| gui
    validate_block1_fail -.-x |notification| gui
    validate_block2_fail -.-x |notification| gui
    subgraph ui
        gui[User interface]
    end

Loading

[nbenn]

I agree with your dislike of

block1 <- data_block(selected = "iris")
block2 <- select_block(data = iris, columns = "Species")

what I was more hoping for is something along the lines of

block1 <- data_block(selected = "iris")
block2 <- select_block(columns = "Species")

being possible.

[nbenn]

Next steps:

1. remove dependencies on data for block constructors
2. get rid of the dual constructor setup, i.e. only have select_block() instead of also having new_select_block()
3. same for field constructors -> constructor should not do initialization/validation for us
4. start removing "fixing things" in the validator functions
5. evaluation stops as soon as block validation fails
   • eval stays in a try-catch
   • an error here has similar consequences as a validation failure
6. ui/communication component

[DivadNojnarg]

All good now with #372