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

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

Closed
wants to merge 41 commits into from

Conversation

slodge
Copy link

@slodge slodge commented Feb 22, 2023

This replaces the previous PRs on type converters, dynamic image plots, dates/datetimes and on open api object schema types - #897 #889 #891 #892

If these changes were ever to go ahead, there's a heap of documentation and some tests to sort out... but that's for future consideration.

Current manual test app:

#
# This is a Plumber API. You can run the API by clicking
# the 'Run API' button above.
#
# Find out more about building APIs with Plumber here:
#
#    https://www.rplumber.io/
#

devtools::load_all()

library(tidyverse)

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


#* Echo back the input
#* @param msg The message to echo
#* @get /echo
function(msg = "") {
  list(msg = paste0("The message is: '", msg, "'"))
}

#* Plot a histogram
#* @serializer png
#* @get /plot
function() {
  rand <- rnorm(100)
  hist(rand)
}

#* Return the sum of two numbers
#* @param a:int The first number to add
#* @param b:int The second number to add
#* @get /sum
function(a, b) {
  message("/sum processing a:", a, " of class ", class(a), " and b:", b, " of class ", class(b))
  a + b
}


#* Return the sum of two sets of numbers
#* @param a:[float] The first set of numbers to add
#* @param b:[float] The second set of numbers to add
#* @get /sumAll
function(a, b) {
  message("/sumAll processing a:", a, " of class ", class(a), " and b:", b, " of class ", class(b))
  sum(a, b)
}

#* Return a filtered set of strings
#* @param a:[logical] The filter map
#* @param b:[string] The set of string to filter
#* @get /filterText
function(a, b) {
  browser()
  message("/filterText processing a:", a, " of class ", class(a), " and b:", b, " of class ", class(b))
  b[a]
}

#* Is this working?
#* @param yes:bool Yes?
#* @get /boolTest
function(yes) {
  browser()
  if (yes) {
    "YES!"
  } else {
    "How sad.."
  }
}

#* @param when:date the date
#* @param t:date-time The date and time
#* @get /dates
function(when, t) {

  browser()
  paste0("The date was ", when, " and the time was ", t)
}

#* Do something
#* @post /doIt
#* @requestBody ARequestThing
#* @response 200 @body AResponseThing The success response
function(req) {
  browser()
  req$body
}

#* This is an object
#* @schema ARequestThing
#* @prop first:string The filter map
#* @prop a:[logical] The filter map
#* @prop b:[string] The set of string to filter
NULL

#* This is an object
#* @schema AResponseThing
#* @prop life:string Just a description
#* @prop details:[object]:AResponseSubThing This gets advanced
NULL

#* This is an object
#* @schema AResponseSubThing
#* @prop id:int Just a description
#* @prop value:string More descriptions
NULL

#* 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")
}

#* Sort letters
#* @param letters:[string]
#* @serializer json
#* @get /letters/<letters:[string]>
function(letters) {
  sort(letters)
}

renders and runs as:
image

slodge and others added 27 commits November 29, 2022 09:14
Co-authored-by: Barret Schloerke <barret@rstudio.com>
Co-authored-by: Barret Schloerke <barret@rstudio.com>
Co-authored-by: Barret Schloerke <barret@rstudio.com>
# Conflicts:
#	R/options_plumber.R
#	man/options_plumber.Rd
# Conflicts:
#	R/openapi-spec.R
#	R/plumber-step.R
@taylo5jm
Copy link

@slodge Thanks for your work on the schemas -- I'm very interested in using this functionality. Is there a way to mark the schema properties as required? Also, anyone here able to give a general status update on this PR? I'm curious when these features might be available. Thanks!

@slodge
Copy link
Author

slodge commented Apr 24, 2023

It looks like I include a line:

      required <- identical(propMat[1,6], "*")

So if you try

#* This is an object
#* @schema ARequestThing
#* @prop first:string* A first property
NULL

then that should hopefully give you a required flag in the spec for first... (not tested this but looks like all the code is there!)


I have no info on when/if any of this will make it into a plumber release. I don't believe there is anyone actively committing any time to plumber within Posit at present, and the team indicated that they won't accept these PR features until they've implemented some new "edition"-based versioning code - so I don't see this happening "soon".

@taylo5jm
Copy link

@slodge Awesome. The * works for me!

Thanks for sharing the status of this PR. The "Schema Object" seems to be a useful component of the OpenAPI Specification (at least in version 3.1.0), so it would be great to see this functionality in plumber.

@schloerke Are you able to provide more color around the issues blocking this PR in terms of timelines?

@slodge
Copy link
Author

slodge commented May 3, 2023

@schloerke @taylo5jm is there any kind of timeline for when someone at Posit might give plumber some attention? If we're waiting on "editions" before accepting new functionality, is there a design or draft PR for those so others can assist getting them built?

@JosiahParry
Copy link
Contributor

I just wanted to follow up on this PR. It would be great to have.

@JosiahParry
Copy link
Contributor

I suspect that this PR is too large to merge? Is there a suggested path forward for us to have argument type specification and parsing without having to manually cast inside the body of functions?

@slodge
Copy link
Author

slodge commented Nov 7, 2023 via email

@JosiahParry
Copy link
Contributor

@slodge thanks! FastAPI is a python library and out of scope of my interest :)

@slodge
Copy link
Author

slodge commented Nov 7, 2023 via email

@CLAassistant
Copy link

CLAassistant commented Nov 8, 2023

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
2 out of 3 committers have signed the CLA.

✅ slodge
✅ schloerke
❌ slodge-work
You have signed the CLA already but the status is still pending? Let us recheck it.

@slodge
Copy link
Author

slodge commented Nov 15, 2023

Notes from #911 also merged

@slodge slodge changed the title Merged: type converters, dynamic image plots, dates/datetimes and on open api object schema types Merged: type converters, dynamic image plots, dates/datetimes, open api object schema types, multiline comments, multipart form fix Nov 15, 2023
@slodge
Copy link
Author

slodge commented Nov 25, 2023

As discussed in #920 this PR is too large and complex to merge.

Respectfully closing.

@slodge slodge closed this Nov 25, 2023
@slodge slodge deleted the ideasMerged branch November 25, 2023 17:25
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

Successfully merging this pull request may close these issues.

6 participants