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

https://github.com/plotly/dashR/blob/6bc44b5c4203b03bd5ef04ab5dd24b23bf9b987b/R/utils.R#L360-L363

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 https://github.com/plotly/dashR/blob/6bc44b5c4203b03bd5ef04ab5dd24b23bf9b987b/R/dash.R#L438-L444

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:

https://github.com/plotly/dashR/blob/6bc44b5c4203b03bd5ef04ab5dd24b23bf9b987b/R/utils.R#L360-L363

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

:one: assert that the argument passed to output is a list :two: assert that this list has two, and only two, elements :three: 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