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

Add sheets_create() #61

Merged
merged 28 commits into from
Nov 22, 2019
Merged

Add sheets_create() #61

merged 28 commits into from
Nov 22, 2019

Conversation

jennybc
Copy link
Member

@jennybc jennybc commented Nov 15, 2019

HERE WE GO 🚀

My next move is to work on the sheets argument .... taking names, taking datasets, etc. But there's already plenty of stuff to decide, so let's get started. I'm going to comment this PR heavily with discussion points.

Below I show a bit of usage.


devtools::load_all("~/rrr/googlesheets4")
#> Loading googlesheets4

sheets_create("sheets-create-demo-1")
#>   Spreadsheet name: sheets-create-demo-1
#>                 ID: 17A4RtHDeEi7L3zBX7FSAXtfhwa-GO3pCalLxCKnLzG4
#>             Locale: en_US
#>          Time zone: Etc/GMT
#>        # of sheets: 1
#> 
#> (Sheet name): (Nominal extent in rows x columns)
#>       Sheet1: 1000 x 26

sheets_create("sheets-create-demo-2", locale = "en_CA")
#>   Spreadsheet name: sheets-create-demo-2
#>                 ID: 1aZqeARCmHXAJcLWmZ5xb4-BhNVHOBxeZLU_tHa3CtEI
#>             Locale: en_CA
#>          Time zone: Etc/GMT
#>        # of sheets: 1
#> 
#> (Sheet name): (Nominal extent in rows x columns)
#>       Sheet1: 1000 x 26

sheets_create(
  "sheets-create-demo-3",
  locale = "fr_FR",
  timeZone = "Europe/Paris"
)
#>   Spreadsheet name: sheets-create-demo-3
#>                 ID: 1LoZnD-swifxuodEDcT4Ta-fVRgKLqVKCOQnTHRLPbgE
#>             Locale: fr_FR
#>          Time zone: Europe/Paris
#>        # of sheets: 1
#> 
#> (Sheet name): (Nominal extent in rows x columns)
#>    Feuille 1: 1000 x 26

Created on 2019-11-15 by the reprex package (v0.3.0.9001)

NAMESPACE Show resolved Hide resolved
@@ -0,0 +1,40 @@
# https://developers.google.com/sheets/api/reference/rest/v4/spreadsheets/other#GridRange
Copy link
Member Author

@jennybc jennybc Nov 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are ~200 so-called schemas in the Sheets API. GridRange is an example.

Once you start writing and updating Sheets, you need to create a lot of bodies, in the REST sense, which was not true while we were only reading. And these bodies must be built up of (often nested) schemas. I have to create some sort of machinery around this.

What I do in this PR would not scale nicely to the full set of schemas, but is that necessary? I don't know. I definitely feel a bit like a schema-and-S3-creating robot, though.

But if I did something more automated re: class creation and validation, then it scales and, in fact, that would belong in gargle, for reuse across multiple APIs.

I don't know what to do.

R/schema_NamedRange.R Outdated Show resolved Hide resolved
R/schema_NamedRange.R Outdated Show resolved Hide resolved
maybe_non_negative_integer(x$sheetId, "sheetId")
maybe_string(x$title, "title")
maybe_non_negative_integer(x$index, "index")
maybe_string(x$sheetType, "sheetType") # enum
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In addition to ~200 schemas there are lots of enums. They aren't particularly easy to pull out of the discovery document in a coherent way. But you could imagine doing so, in a fit of recursive fury, and create a way to declare something a enum, check supplied values against the valid ones, etc.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Auto-generating an internal data structure and code around the enums sounds much easier to me (maybe even a clear "yes, do this"?) than doing same for the schemas.

@jennybc jennybc requested a review from hadley November 15, 2019 18:38
R/schema_Sheet.R Outdated Show resolved Hide resolved
@jennybc
Copy link
Member Author

jennybc commented Nov 16, 2019

OK the constructors for S3 classes based on schemas are very minimalist now.

I must confess I don't see any reason to write these constructors. It feels like I could accomplish the same automatically based on the internal .schemas object, i.e. via a hypothetical new_from_schema(schema) function. All they do is construct a list with NULL elements with specific names. But I will trust in the process.

@hadley
Copy link
Member

hadley commented Nov 16, 2019

Maybe you shouldn’t then? Maybe it doesn’t make sense to use S3 here at all, and you could just constructor a validation function from the spec?

Only change was correcting an instance of camelCase to snake_case in a description.
Various things just seem to be in a different order
If these "row order diffs" are a recurring phenomenon, I'll have to do something ... like sort properties alphabetically, inside schema_rectangle()
Everytime I re-download the JSON, the order changes. So I will definitely arrange() before I write my diffable objects.
@jennybc jennybc merged commit b92372c into master Nov 22, 2019
@jennybc jennybc deleted the sheets-create branch February 18, 2020 05:48
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.

2 participants