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

Improved documentation on specifying param type #755

Open
JosiahParry opened this issue Jan 30, 2021 · 6 comments
Open

Improved documentation on specifying param type #755

JosiahParry opened this issue Jan 30, 2021 · 6 comments

Comments

@JosiahParry
Copy link
Contributor

The plumber documentation on specifying input parameters is limited (see https://www.rplumber.io/articles/annotations.html#more-details-on-param).

Take the following example:

#* @param double:dbl
#* @get /test
function(double = 0) {
  class(double)
}

When using the swagger interface with double set to 0.5 the response is

[
  "character"
]

The only way to achieve the type specification is with an override from R

#* @param double:dbl
#* @get /override
function(double = 0) {
  class(as.double(0))
}

Which returns

[
  "numeric"
]

My intention with this example is that, given the documentation, it is not clear how to specify input types.
I am using plumber v1.0.0

@meztez
Copy link
Collaborator

meztez commented Jan 31, 2021

I agree. When I first started using plumber I was confused why plumber did not do this conversion.

Plumber will do the conversion in the case of dynamic path parameter.

#* @get /test/<double:dbl>
function(double = 0) {
  class(double)
}

which you call using host/test/0.5

But not in the case of query parameters, query parameters will always be sent back to the API as character. The only reason to define the type in this case is for the OpenAPI document generation.

#* @param double:dbl
#* @get /test
function(double = 0) {
  class(double)
}

I think I had this explained to me at some point. I'll take a look if I can find it. Maybe specify it in the documentation.

@JosiahParry
Copy link
Contributor Author

This has come up again in a (inflamitory, imo 😉 ) blog post.

They make a valid point that the param type annotations are not checked or enforced.
Reprex

library(plumber)

#* @param n:int
#* @get /types
function(n) {
  n  * 2L
}

In browser make a request with any valid n and the response in the R client is <simpleError in n * 2: non-numeric argument to binary operator>

Assertions and coersions do not work well here becuase as.integer("asdf") returns NA_integer_ which, if passed into a type check for stopifnot(is.integer(as.integer(n)) would return TRUE.

@JosiahParry
Copy link
Contributor Author

If you can point me where to most likely more a change here, I'd be happy to submit a PR.

@schloerke
Copy link
Collaborator

@JosiahParry Check out vignettes/annotations.Rmd. Thank you!

@JosiahParry
Copy link
Contributor Author

Yes, the important part here is:

Query parameters currently need to be explicitly converted as they are pushed as is (character) to block expression. Only dynamic route parameters are converted to specified @param type before being pushed to block expression.

The following endpoint will correctly do type conversion:

#* @param n
#* @get /type/<n:int>
function(n) {
  n  * 2L
}

How can we get this same behavior for non-dynamic routes?

@schloerke
Copy link
Collaborator

@JosiahParry I believe this PR has it done: #905 .

I'll start carving away time to get this through.

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

No branches or pull requests

3 participants