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 tags to metadata #677

Merged
merged 9 commits into from
Dec 6, 2022
Merged

Add tags to metadata #677

merged 9 commits into from
Dec 6, 2022

Conversation

juliasilge
Copy link
Member

Closes #570

This PR adds a tags field to pin metadata. It is used like this:

library(pins)
b <- board_rsconnect()
#> Connecting to RSC 2022.09.0 at <https://colorado.rstudio.com/rsc>
b %>% pin_write(
  1:10, 
  "julia.silge/great-numbers", 
  tags = c("i-love-integers", "numbers-are-fun")
)
#> Guessing `type = 'rds'`
#> Writing to pin 'julia.silge/great-numbers'

Created on 2022-11-16 with reprex v2.0.2

You can see how this is stored on Connect here (click "Raw metadata" to see the tags stored in YAML).

This adds a new argument in pin_write() between description and metadata, which could cause problems for any folks who have used metadata by position only. It does not fail silently, though:

library(pins)
b <- board_temp()
b %>% pin_write(1:10, "great-numbers", "json", "My beautiful numbers", "Some numbers for you to enjoy", list(a = "a"))
#> Error in `check_tags()` at pins-r/R/pin-read-write.R:88:2:
#> ! `tags` must be a character vector

#> Backtrace:
#>     ▆
#>  1. ├─b %>% ...
#>  2. └─pins::pin_write(...)
#>  3.   └─pins:::check_tags(tags) at pins-r/R/pin-read-write.R:88:2
#>  4.     └─rlang::abort("`tags` must be a character vector") at pins-r/R/pin-read-write.R:256:4

Created on 2022-11-16 with reprex v2.0.2

I think the number of folks who have used metadata like this (by position) must be very small, though, because there are so many arguments before it. Is this a breaking change? Probably?

For future extensions, on Connect we could try to store these tags as Connect tags. Publishers cannot create tags, so the plan would be to check if the tag exists and then add it to the pin.

@juliasilge
Copy link
Member Author

This change is backwards compatible for reading pins that were stored without tags:

library(pins)
b <- board_rsconnect()
#> Connecting to RSC 2022.09.0 at <https://colorado.rstudio.com/rsc>
b %>% pin_meta("julia.silge/wow-another-mtcars-model")
#> List of 11
#>  $ file       : chr "wow-another-mtcars-model.rds"
#>  $ file_size  : 'fs_bytes' int 5.07K
#>  $ pin_hash   : chr "ebc45bb5da89fbcc"
#>  $ type       : chr "rds"
#>  $ title      : chr "wow-another-mtcars-model: a pinned list"
#>  $ description: chr "An OLS linear regression model"
#>  $ created    : POSIXct[1:1], format: "2022-10-07 14:45:38"
#>  $ api_version: num 1
#>  $ user       : list()
#>  $ name       : chr "julia.silge/wow-another-mtcars-model"
#>  $ local      :List of 4
#>   ..$ dir       : 'fs_path' chr "~/Library/Caches/pins/rsc-e62371cfd77db754024f9c5ed3556a73/3ff71b31-f257-480a-a87b-fc6dc9a7e4dc/63410"
#>   ..$ url       : chr "https://colorado.rstudio.com/rsc/content/3ff71b31-f257-480a-a87b-fc6dc9a7e4dc/_rev63410/"
#>   ..$ version   : chr "63410"
#>   ..$ content_id: chr "3ff71b31-f257-480a-a87b-fc6dc9a7e4dc"

Created on 2022-11-16 with reprex v2.0.2

@juliasilge juliasilge requested a review from machow November 16, 2022 21:01
@juliasilge
Copy link
Member Author

@machow do you mind taking a look, with an eye to a) whether this change would be a problem for pins for Python as it exists today and b) whether you think this approach would work for Python (later, when adding it may be appropriate)?

Copy link
Collaborator

@machow machow left a comment

Choose a reason for hiding this comment

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

This LGTM, and should be pretty quick to add to python pins!

I left a quick question on whether tags should be added as the last argument to pin_write. It seems like "tags" would be pretty quick to type for kw args, and will let this be backwards compatible (but I get wanting it to come before rarely used arguments 😭)

R/pin-read-write.R Outdated Show resolved Hide resolved
@juliasilge
Copy link
Member Author

juliasilge commented Nov 30, 2022

I think you are right @machow that we should just put this at the end. I still kept it before the dots ... which means it could possibly still be a breaking change for folks who have used all existing args by position only (not by name) and then passed something through the dots; I think this must be very rare since there are so many arguments ahead of the dots.

@juliasilge juliasilge requested a review from hadley November 30, 2022 16:07
@@ -60,6 +60,8 @@ pin_read <- function(board, name, version = NULL, hash = NULL, ...) {
#' lists and rds for everything else.
#' @param versioned Should the pin be versioned? The default, `NULL`, will
#' use the default for `board`
#' @param tags A character vector of tags for the pin; most important for
#' discoverability on shared boards.
#' @rdname pin_read
#' @export
pin_write <- function(board, x,
Copy link
Member

Choose a reason for hiding this comment

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

If I was to design this function today, I would put ... after x.

R/pin-read-write.R Outdated Show resolved Hide resolved
@juliasilge juliasilge merged commit 95af558 into main Dec 6, 2022
@juliasilge juliasilge deleted the metadata-tags branch December 6, 2022 02:23
@github-actions
Copy link

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.

@github-actions github-actions bot locked and limited conversation to collaborators Dec 21, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add tags field to metadata
3 participants