-
Notifications
You must be signed in to change notification settings - Fork 62
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
Revamp board_url()
to handle manifest files
#681
Conversation
Merge commit '5dec8da719be63fb260d81e447c54a3fc463df03' #Conflicts: # R/board_url.R # tests/testthat/test-board_url.R
I was having a hard time with the tests until I remembered about |
@ijlyttle do you have time to install this and check it out? Also give me any feedback or thoughts on the code? |
Hi @juliasilge - this is great news! I'll go through this today! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @juliasilge - this looks great!
I also took it for a spin and it works just as I expect. I left a few very picky comments, most of it deals with faults in the original material.
As an aside, the organization is much improved from the original PR :)
Thanks so much for taking on these strange ideas and for being patient with my "first-draft" code. I remain at your service to "help" with the Python side!
Thank you again so much @ijlyttle! 🙌 I really appreciate you being willing to put your thoughts down in the draft PR, and for reviewing. It did end up being a fairly significant set of changes so I'm glad we broke it up! A question for you ❓ |
It would be my pleasure to contribute such a vignette! If you like, I'll wait for the dust to settle on this PR, then start another PR. |
Co-authored-by: Hadley Wickham <[email protected]>
Merge branch 'board-url-versioned' of https://github.com/rstudio/pins-r into board-url-versioned # Conflicts: # R/board_url.R
#' - Unnamed character scalar, i.e. **a single URL** to a | ||
#' [manifest file][write_board_manifest()]: If the URL ends in a `/`, | ||
#' `board_url()` will look for a `_pins.yaml` manifest. If the manifest | ||
#' file parses to a named list, versioning is supported. If it parses to a | ||
#' named character vector, the board will not support versioning. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why would you point it to something other than a directory?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is possible to pass in a URL to a file that is a manifest file but not named _pins.yaml
, like this:
library(pins)
b1 <- board_folder(withr::local_tempdir(), versioned = TRUE)
b1 %>% pin_write(1:10, "x", type = "json")
#> Creating new version '20221215T205631Z-c3943'
#> Writing to pin 'x'
b1 %>% pin_write(11:20, "y", type = "json")
#> Creating new version '20221215T205631Z-cba09'
#> Writing to pin 'y'
b1 %>% pin_write(1:20, "y", type = "csv")
#> Creating new version '20221215T205631Z-5026d'
#> Writing to pin 'y'
write_board_manifest(b1)
#> Manifest file written to root folder of board, as `_pins.yaml`
fs::file_move(fs::path(b1$path, "_pins.yaml"), fs::path(b1$path, "_different.yaml"))
b1_path <- fs::path(b1$path)
b1_server <- webfakes::new_app()
b1_server$use(webfakes::mw_static(root = b1_path))
b1_process <- webfakes::new_app_process(b1_server)
b2 <- board_url(fs::path(b1_process$url(), "_different.yaml"))
b2 %>%
pin_read("x")
#> → <http:/127.0.0.1:64054/x/20221215T205631Z-c3943/data.txt> is not cacheable
#> → <http:/127.0.0.1:64054/x/20221215T205631Z-c3943/x.json> is not cacheable
#> [1] 1 2 3 4 5 6 7 8 9 10
Created on 2022-12-15 with reprex v2.0.2
I'm sure it won't be the most common use case but seems like some nice flexibility to offer.
Co-authored-by: Hadley Wickham <[email protected]>
Co-authored-by: Hadley Wickham <[email protected]>
This pull request has been automatically locked. If you believe you have found a related problem, please file a new issue (with a reprex: https://reprex.tidyverse.org) and link to this issue. |
Related to #661
Closes #650