Dynamic Plot Endpoints #897

slodge · 2023-01-04T16:44:42Z

Pull Request

First attempt at #837

Still to do:

Lots of QA - both manual and unit tests
Lots of Docs
Merging with the other open PRs - this is going to be a bit of a mess now :/
News.md
devtools::Check Package
Review
Possibly more granular control on parameters - do we want to turn things on/off per endpoint? do we want more options? (e.g. quality)? do we want less?

To consider

breaking backward compatibility - don't think it does - but it's possible...
is hard to understand - possibly
is hard to maintain in the future - possibly ... but no worse than the rest of plumber? there's some technical debt building up in the open api params I think

Minimal reproducible example

Have run:

devtools::load_all()
library(tidyverse)

#* @apiTitle Plumber Example API
#* @apiDescription Plumber example description.

#* Plot out data from the iris dataset
#* @param spec If provided, filter the data to only this species (e.g. 'setosa')
#* @get /plot
#* @serializer png
function(spec = "setosa"){
  myData <- iris
  title <- "All Species"

  # Filter if the species was specified
  if (!missing(spec)){
    title <- paste0("Only the '", spec, "' Species")
    myData <- subset(iris, Species == spec)
  }

  plot(myData$Sepal.Length, myData$Petal.Length,
       main=title, xlab="Sepal Length", ylab="Petal Length")
}

This shows as:

and:

Seems to work... 👍

CLAassistant · 2023-01-04T16:44:47Z

All committers have signed the CLA.

…onnected)

schloerke · 2023-01-04T17:30:54Z

R/serializer.R

  }

+  dev_requires_req <- "req" %in% names(formals(dev_on))


Adding validations on dev_on()

** Untested

Suggested change

dev_requires_req <- "req" %in% names(formals(dev_on))

dev_on_arg_names <- names(formals(dev_on))

dev_on_requires_req <- "req" %in% dev_on_arg_names

# Make sure extra args exist

if (dev_on_requires_req) {

dots_pos <- which("..." %in% dev_on_arg_names

if (length(dots_pos) == 0) stop("`dev_on()` must contain arguments `...` if using `req`")

req_pos <- which("..." %in% dev_on_arg_names

if (!isTRUE(dots_pos < req_pos)) {

stop("`dev_on()` must have arguments `...` before `req=`)

}

}

Thanks for all the immediate feedback...

Given how much there is, it might be an idea to make and edit these changes in an editor and to add some unit tests as we go?

If it helps, we can hold off on this (and the other PRs) until RStudio have capacity to invest the time? (That might also help me negotiate some wriggle room with my work pressures too!)

schloerke · 2023-01-04T17:32:11Z

R/serializer.R

+  ignored <- Map(function(x) {
+    params[[x]] <<- NULL
+  }, all_except)


This might work?

** Untested

Suggested change

ignored <- Map(function(x) {

params[[x]] <<- NULL

}, all_except)

params[all_except] <- NULL

R/openapi-spec.R

schloerke · 2023-01-04T17:34:56Z

R/options_plumber.R

+#' \item{`plumber.staticSerializers`}{Plumber will use fixed serializers and
+#' will not interpret e.g. plot_width and plot_height parameters as plot image
+#' size instructions}


Suggested change

#' \item{`plumber.staticSerializers`}{Plumber will use fixed serializers and

#' will not interpret e.g. plot_width and plot_height parameters as plot image

#' size instructions}

#' \item{`plumber.staticSerializers`}{If `TRUE`, Plumber will use fixed serializers and

#' will not interpret e.g. `plot_width` and `plot_height` parameters as plot image

#' size instructions. Defaults to `FALSE`}

schloerke · 2023-01-04T17:35:18Z

R/serializer.R

@@ -446,6 +446,7 @@ serializer_xml <- function() {
 #' @param preexec_hook Function to be run directly before a [PlumberEndpoint] calls its route method.
 #' @param postexec_hook Function to be run directly after a [PlumberEndpoint] calls its route method.
 #' @param aroundexec_hook Function to be run around a [PlumberEndpoint] call. Must handle a `.next` argument to continue execution. \lifecycle{experimental}
+#' @param serializer_params Dynamic serializer parameters. More docs needed here!


Suggested change

#' @param serializer_params Dynamic serializer parameters. More docs needed here!

#' @param serializer_params Dynamic serializer parameters. For internal use.

schloerke · 2023-01-04T17:35:35Z

R/serializer.R

@@ -497,26 +501,37 @@ self_set_serializer <- function(self, serializer) {
 #' The graphics device `dev_on` function will receive any arguments supplied to the serializer in addition to `filename`.
 #' `filename` points to the temporary file name that should be used when saving content.
 #' @param dev_off Function to turn off the graphics device. Defaults to [grDevices::dev.off()]
+#' @param serializer_params More docs needed here


schloerke · 2023-01-04T17:36:18Z

R/serializer.R

  stopifnot(is.function(dev_off))

  endpoint_serializer(
    serializer = serializer_content_type(type),
    aroundexec_hook = function(..., .next) {
      tmpfile <- tempfile()

-      dev_on(filename = tmpfile)
+      if (dev_requires_req) {


(Using larger name from above suggestion)

Suggested change

if (dev_requires_req) {

if (dev_on_requires_req) {

schloerke · 2023-01-04T17:36:57Z

R/serializer.R

+  list(plot_units = list(desc="Units of plot image", type="string", required=FALSE, isArray=FALSE))
+}
+serializer_param_pointsize <- function() {
+  list(plot_pointsize = list(desc="Point size of plot image - TODO - better desc", type="number", required=FALSE, isArray=FALSE))


schloerke · 2023-01-04T17:38:55Z

R/serializer.R

+
+  # if legacy plumber.staticSerializers is specified then ignore all req
+  # based parameters
+  if (getOption("staticSerializers", default="FALSE") == "TRUE") {


Same change as above

Suggested change

if (getOption("staticSerializers", default="FALSE") == "TRUE") {

if (isTRUE(getOption("plumber.staticSerializers", default=FALSE))) {

schloerke · 2023-01-04T17:40:16Z

R/serializer.R

+    if (is.null(option)) {
+      NULL
+    } else {
+      as.numeric(option)
+    }


Cosmetic

Suggested change

if (is.null(option)) {

NULL

} else {

as.numeric(option)

}

if (is.null(option)) return(NULL)

as.numeric(option)

schloerke · 2023-01-04T17:45:25Z

R/serializer.R

+    dimension_args <- serialize_dimensions_args_preparer(req, ...)
+    rlang::exec(grFunc, filename, !!!dimension_args)


Cosmetic

Suggested change

dimension_args <- serialize_dimensions_args_preparer(req, ...)

rlang::exec(grFunc, filename, !!!dimension_args)

dev_args <- serialize_dev_args_preparer(req, ...)

rlang::exec(grFunc, filename, !!!dev_args)

schloerke · 2023-01-04T17:45:41Z

R/serializer.R

+}
+
+
+serialize_dimensions_args_preparer <- function(req, ...) {


Matching cosmetic change below

Suggested change

serialize_dimensions_args_preparer <- function(req, ...) {

serialize_dev_args_preparer <- function(req, ...) {

schloerke · 2023-01-04T17:51:12Z

R/serializer.R

+  doc_args$width <- as_numeric_nullable(req$args$plot_width, doc_args$width)
+  doc_args$height <- as_numeric_nullable(req$args$plot_height, doc_args$height)
+  doc_args$units <- req$args$plot_units %||% doc_args$units
+  doc_args$res <- as_numeric_nullable(req$args$plot_res, doc_args$res)
+  doc_args$pointsize <- as_numeric_nullable(req$args$plot_pointsize, doc_args$pointsize)
+  doc_args$bg <- req$args$plot_bg %||% doc_args$bg


What is your motivation to use plot_* for the arg names?
Why not pass them through directly?

My interpretation:

Minimal param clashing with plot_*

Confusion of two differently named parameters

Params in @serializer png list(width=200)

Params in http://....../plot_route?plot_width=200

If there isn't too much motivation for using plot_* names, I'd like to keep the consistent arg names when possible. (Knowing there could be param name clashing with the regular route params)

As well as avoiding clashes, my main motivation behind plot_ names was to make it easier to distinguish the plot parameters from the business logic - e.g. in https://.../palmerPenguins?min_flipper_length=200&min_bill_length=40&plot_width=400&plot_height=500

The naming also makes the business logic vs plot parameter separation obvious in the Swagger UI (maybe some more ordering there might also help?)

e.g. it's obvious here which params are business vs which are plot:

Asides:

initially I was wondering about using e.g. plot.width to try to make the plot parameters look like a separate object in the REST url (but couldn't spot any docs on that in https://swagger.io/docs/specification/describing-parameters/)

also wondering whether we should allow HTTP header insertion of these parameters (not sure that's very RESTFUL though)

schloerke · 2023-01-04T17:54:55Z

R/openapi-spec.R

+  # if legacy plumber.staticSerializers is specified then ignore all req serializer
+  # based parameters
+  if (getOption("plumber.staticSerializers", default="FALSE") == "TRUE") {
+    serializerParams <- list()
+  } else {
+    # Get the plumber serializer defined endpoint params
+    serializerParams <- routerEndpointEntry$getSerializerParams()
+  }


Let's let the endpoint determine it's serializer params.

This will allow for the endpoint to have full control vs using a globally set flag.

Counter example app: API with two routes: 1 w/ dynamic serializer and 1 w/ static serializer.

Suggested change

# if legacy plumber.staticSerializers is specified then ignore all req serializer

# based parameters

if (getOption("plumber.staticSerializers", default="FALSE") == "TRUE") {

serializerParams <- list()

} else {

# Get the plumber serializer defined endpoint params

serializerParams <- routerEndpointEntry$getSerializerParams()

}

# Get the plumber serializer defined endpoint params

serializerParams <- routerEndpointEntry$getSerializerParams()

schloerke · 2023-01-04T19:17:43Z

R/plumber-step.R

+    #' @description retrieve serializer endpoint parameters
+    getSerializerParams = function() {
+      if (is.null(self$serializer_params)) {
+        return(list())
+      }
+      return(self$serializer_params)
+    },


Suggested change

#' @description retrieve serializer endpoint parameters

getSerializerParams = function() {

if (is.null(self$serializer_params)) {

return(list())

}

return(self$serializer_params)

},

#' @description retrieve serializer endpoint parameters

getSerializerParams = function() {

self$serializer_params %||% list()

},

schloerke · 2023-01-04T19:19:58Z

R/serializer.R

 #' @export
-serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off) {
+serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = list()) {


Let's use a rlang::missing_arg() as a placeholder and handle it if it is missing via the option value.

Suggested change

serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = list()) {

serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = missing_arg()) {

schloerke · 2023-01-04T19:20:09Z

R/serializer.R

 #' @export
-serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off) {
+serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = list()) {


Let's use a rlang::missing_arg() as a placeholder and handle it if it is missing via the option value.

Suggested change

serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = list()) {

serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = missing_arg()) {

schloerke · 2023-01-04T19:23:59Z

R/serializer.R

  }

+  dev_requires_req <- "req" %in% names(formals(dev_on))
+
  stopifnot(is.function(dev_off))

  endpoint_serializer(


Handling the serializer_params

** Untested

Suggested change

endpoint_serializer(

serializer_params <- rlang::maybe_missing(serializer_params, {

# Legacy support for static serializer parameters

if (isTRUE(getOption("plumber.staticSerializers", default=FALSE)) return(list())

# All known parameters for devices

serializer_param_list()

})

endpoint_serializer(

schloerke · 2023-01-04T19:27:36Z

R/plumber-step.R

+    #' @field serializer_params serializer parameters
+    serializer_params = NULL,


As much as I don't like it, let's make the camelCase and change to dynamicSerializerParams (or something to hint that it is the dynamic serializer params... dynSerParams?)

Will need to change all usage as well. :-/

schloerke · 2023-01-04T19:37:43Z

R/serializer.R

 serializer_jpeg <- function(..., type = "image/jpeg") {
  serializer_device(
    type = type,
-    dev_on = function(filename) {
-      grDevices::jpeg(filename, ...)
-    }
+    dev_on = serializer_image_dev_on_func(grDevices::jpeg, ...),
+    serializer_params = serializer_param_list()
  )
 }


We should allow users to determine if the dynamic params are used as the serializer level.

Maybe something like:

get_serializer_params <- function(dynamic_params) { has_dynamic_params <- rlang::maybe_missing(dynamic_params, { # Legacy support for static serializer parameters if (isTRUE(getOption("plumber.staticSerializers", default=FALSE)) return(FALSE) # Support dynamic params TRUE }) if (!has_dynamic_params) return(list()) serializer_param_list() } serializer_jpeg <- function(..., type = "image/jpeg", dynamic_params = missing_arg()) { serializer_device( type = type, dev_on = serializer_image_dev_on_func(grDevices::jpeg, ...), serializer_params = get_serializer_params(dynamic_params) ) }

Will need to document the dynamic_params parameter.

slodge · 2023-02-22T17:45:01Z

Closing - too hard to maintain long running (and stale) PRs

Dynamic Parameters (third attempt at getting my github user account c…

77901fd

…onnected)

slodge force-pushed the dynamic_plots branch from d204549 to 77901fd Compare January 4, 2023 16:51

slodge changed the title ~~Dynamic endpoints~~ Dynamic Plot Endpoints Jan 4, 2023

missed some serializer params

4c9a87a

schloerke reviewed Jan 4, 2023

View reviewed changes

R/openapi-spec.R Show resolved Hide resolved

schloerke reviewed Jan 4, 2023

View reviewed changes

slodge mentioned this pull request Jan 12, 2023

Docs (or helper functions?) on consuming Plumber APIs in R, Shiny and RMarkdown/Quarto #901

Open

slodge closed this Feb 22, 2023

slodge mentioned this pull request Feb 22, 2023

Merged: type converters, dynamic image plots, dates/datetimes, open api object schema types, multiline comments, multipart form fix #905

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Dynamic Plot Endpoints #897

Dynamic Plot Endpoints #897

slodge commented Jan 4, 2023

CLAassistant commented Jan 4, 2023 •

edited

Loading

schloerke Jan 4, 2023

slodge Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023 •

edited

Loading

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

slodge Jan 4, 2023 •

edited

Loading

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

schloerke Jan 4, 2023

slodge commented Feb 22, 2023

-  dev_requires_req <- "req" %in% names(formals(dev_on))
+  dev_on_arg_names <- names(formals(dev_on))
+  dev_on_requires_req <- "req" %in% dev_on_arg_names
+  # Make sure extra args exist
+  if (dev_on_requires_req) {
+    dots_pos <- which("..." %in% dev_on_arg_names
+    if (length(dots_pos) == 0) stop("`dev_on()` must contain arguments `...` if using `req`")
+    req_pos <- which("..." %in% dev_on_arg_names
+    if (!isTRUE(dots_pos < req_pos)) {
+      stop("`dev_on()` must have arguments `...` before `req=`)
+    }
+  }

	#' @param serializer_params Dynamic serializer parameters. More docs needed here!
	#' @param serializer_params Dynamic serializer parameters. For internal use.

	if (getOption("staticSerializers", default="FALSE") == "TRUE") {
	if (isTRUE(getOption("plumber.staticSerializers", default=FALSE))) {

		dimension_args <- serialize_dimensions_args_preparer(req, ...)
		rlang::exec(grFunc, filename, !!!dimension_args)

	serialize_dimensions_args_preparer <- function(req, ...) {
	serialize_dev_args_preparer <- function(req, ...) {

	serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = list()) {
	serializer_device <- function(type, dev_on, dev_off = grDevices::dev.off, serializer_params = missing_arg()) {

-  endpoint_serializer(
+  serializer_params <- rlang::maybe_missing(serializer_params, {
+      # Legacy support for static serializer parameters
+      if (isTRUE(getOption("plumber.staticSerializers", default=FALSE)) return(list())
+      # All known parameters for devices
+      serializer_param_list()
+    })
+  endpoint_serializer(

		#' @field serializer_params serializer parameters
		serializer_params = NULL,

Dynamic Plot Endpoints #897

Dynamic Plot Endpoints #897

Conversation

slodge commented Jan 4, 2023

Pull Request

Still to do:

To consider

Minimal reproducible example

CLAassistant commented Jan 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

schloerke Jan 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slodge Jan 4, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slodge commented Feb 22, 2023

CLAassistant commented Jan 4, 2023 •

edited

Loading

schloerke Jan 4, 2023 •

edited

Loading

slodge Jan 4, 2023 •

edited

Loading