Revise assert_valid_callbacks to permit passing unnamed lists as callback output

[z267xu]

In Python, we may not need to give names to a list. In R, we would better do so, or the extraction may return NULL. The output is a list with first argument id and second property. Hence, if the users fail to give names, the following line would throw an error

dashR/R/utils.R

Lines 360 to 363 in 6bc44b5

	# Assert that the component ID as passed is a string.
	if(!(is.character(output$id) & !grepl("^\\s*$", output$id) & !grepl("\\.", output$id))) {
	stop(sprintf("Callback IDs must be (non-empty) character strings that do not contain one or more dots/periods. Please verify that the component ID is valid."), call. = FALSE)
	}

So, we can add an outputCheck function bellow.

outputCheck <- function(output) {
  
  if(length(output) != 2) {
    
    stop(sprintf("Both 'id' and 'property' should be provided"), call. = FALSE)
    
  } else {
    
    namesOutput <- names(output)
    
    if(is.null(namesOutput)) {
      
      warning(sprintf("output names are missing. The first element in output would be set as 'id' and the second would be 'property'"), call. = FALSE)
      
      names(output) <- c("id", "property")
      
    } else {
      
      if(!all(namesOutput %in% c("id", "property"))) {
        
        warning(sprintf("One of output names is missing."), call. = FALSE)
        
        names(output) <- if(all(namesOutput == c("", "property")) | all(namesOutput == c("id", ""))) {

          c("id", "property") 
          
        } else if(all(namesOutput == c("property", "")) | all(namesOutput == c("", "id"))) {
          
          c("property", "id")
          
        } else stop(sprintf("Only 'id' and 'property' can be provided"))
        
      } else if (all(namesOutput %in% "id")) {
        
        warning(sprintf("Both names are 'id', the second one would be set as 'property'"))
        
        names(output) <- c("id", "property")
        
      } else if (all(namesOutput %in% "property")) {
        
        warning(sprintf("Both names are 'property', the first one would be set as 'id'"))
        
        names(output) <- c("id", "property")
        
      } else NULL
    }
  }
  
  output
}

and draw this line

output <- outputCheck(output)

in

dashR/R/dash.R

Lines 438 to 444 in 6bc44b5

	callback = function(output, params, func) {
	assert_valid_callbacks(output, params, func)
	inputs <- params[vapply(params, function(x) 'input' %in% attr(x, "class"), FUN.VALUE=logical(1))]
	state <- params[vapply(params, function(x) 'state' %in% attr(x, "class"), FUN.VALUE=logical(1))]
	# register the callback_map

before assert_valid_callbacks(output, params, func)

It has been tested and should handle all kinds of missing given names
@rpkyle

[rpkyle]

Thanks @z267xu -- nice find and great first issue!

This is definitely what I would call a bug, since we want to permit app developers to use similar shorthand in DashR as in Dash for Python. For example, either of the following is acceptable callback syntax:

@app.callback(
    Output(component_id='my-div', component_property='children'),

@app.callback(
    Output('my-div', 'children'),

Given the way assert_valid_callbacks is currently written, the following line fails in DashR:

app$callback(output = list("my-div", "children"), 

with the error

Error in if (!(is.character(output$id) & !grepl("^\\s*$", output$id) &  : 
  argument is of length zero

The reason for this is that the corresponding test in assert_valid_callbacks expects a named list for output, which is actually not idiomatic for Dash:

dashR/R/utils.R

Lines 360 to 363 in 6bc44b5

	# Assert that the component ID as passed is a string.
	if(!(is.character(output$id) & !grepl("^\\s*$", output$id) & !grepl("\\.", output$id))) {
	stop(sprintf("Callback IDs must be (non-empty) character strings that do not contain one or more dots/periods. Please verify that the component ID is valid."), call. = FALSE)
	}

I believe this test should actually (and in this order)

1️⃣ assert that the argument passed to output is a list
2️⃣ assert that this list has two, and only two, elements
3️⃣ treat output[[1]] as id, and output[[2]] as its property, them perform same check as before

I'm not opposed to writing additional tests to inspect the output, but I'd rather keep it simple whenever possible.

@alexcjohnson