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

Csv serializer #520

Merged
merged 25 commits into from
Jun 23, 2020
Merged

Csv serializer #520

merged 25 commits into from
Jun 23, 2020

Conversation

pachadotdev
Copy link
Contributor

Pull Request

Brief description of the original problem and the approach behind your solution: different forums suggest to write to a temp file and then use readLines to serialize csv. My approach uses readr:: to do that in a more efficient way and serialize without problem.

Here'a a MWE that returns CSV output with correct type:

test-csv-serializer.R

api <- plumber::plumb("return-mtcars.R")
api$run(port = 8080, host = "0.0.0.0")

return-mtcars.R

#' test mtcars
#' @serializer csv
#' @get /
function(res){
  mtcars
}

R/serializer-csv.R Outdated Show resolved Hide resolved
@schloerke schloerke added this to the v0.5.0 - Next CRAN release milestone Apr 7, 2020
DESCRIPTION Outdated Show resolved Hide resolved
R/serializer-csv.R Outdated Show resolved Hide resolved
@pachadotdev
Copy link
Contributor Author

@schloerke hi, any updates on this?

R/serializer-csv.R Outdated Show resolved Hide resolved
Pachamaltese and others added 4 commits June 18, 2020 11:51
Co-authored-by: Barret Schloerke <barret@rstudio.com>
Co-authored-by: Barret Schloerke <barret@rstudio.com>
Co-authored-by: Barret Schloerke <barret@rstudio.com>
Co-authored-by: Barret Schloerke <barret@rstudio.com>
@pachadotdev
Copy link
Contributor Author

@schloerke thanks for the lower/uppercase change
I see it's passing ubuntu tests on gh-actions

@schloerke

This comment has been minimized.

@schloerke
Copy link
Collaborator

Notes for me for later: This follows pattern with serializer-htmlwidget.R

@schloerke
Copy link
Collaborator

/document

DESCRIPTION Outdated Show resolved Hide resolved
@pachadotdev

This comment has been minimized.

@CLAassistant
Copy link

CLAassistant commented Jun 18, 2020

CLA assistant check
All committers have signed the CLA.

@pachadotdev
Copy link
Contributor Author

@schloerke all set, it only asks for runner to sign the CLA

@pachadotdev
Copy link
Contributor Author

@schloerke all set, it only asks for runner to sign the CLA

btw, this is being used to provide csv data for scientists doing covid research in my country, this PR makes it more formal :)

@schloerke
Copy link
Collaborator

@pachamaltese Thank you for the quick updates today. I'll be looking at later today with a coworker.

asks for runner to sign the CLA

This was my fault. Sorry about that.

btw, this is being used to provide csv data for scientists doing covid research in my country, this PR makes it more formal :)

Very neat! If there is anything public, feel free to direct me to it. 😃

@pachadotdev
Copy link
Contributor Author

pachadotdev commented Jun 18, 2020

@schloerke welcome!
yes, here's the result
API: https://coronavirus-api.mat.uc.cl/
docs: https://github.com/pachamaltese/api-covid19-datauc
dashboard: https://coronavirus.mat.uc.cl/

it adds new data daily, thanks to CRON + automated R scripts

@schloerke schloerke requested a review from cpsievert June 18, 2020 20:33
@schloerke schloerke merged commit e535922 into rstudio:master Jun 23, 2020
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.

4 participants