search

rstudio / pins-r

Pin, discover, and share resources
https://pins.rstudio.com
Other
312 stars 63 forks source link

Replacing pin_created with pin_hash #595

Closed thomaszwagerman closed 2 years ago

thomaszwagerman commented 2 years ago

Replaces pin_created() with pin_hash(), which supports functionality of pin_reactive_read() and pin_reactive_download().

Suggestion made by @TylerGrantSmith.

Closes #542

thomaszwagerman commented 2 years ago

Looking at this again and had a thought.

I've replaced pin_created() with a function called pin_hash(), however where pin_meta() returns $created for the date the pin was created, for the hash of the pin contents it returns $pin_hash. When I looked back over the code this caused confusion.

Either I change the pin_hash() function name, or the output of pin_meta() should be changed from $pin_hash -> $hash, which I think is more consistent with the rest of pin_meta()'s outputs.

juliasilge commented 2 years ago

Thank you so much for this PR @thomaszwagerman and for your patience on it!

I agree that it's not ideal that the hash is called pin_hash in the pin metadata. However, it wouldn't really solve this specific question because there is an existing unexported function called pin_hash():

https://github.com/rstudio/pins-r/blob/9d014de4d9384213ebc4e6e02888de5eb36d4ffd/R/pin-read-write.R#L209-L216

Even if we were to change the name of pin_hash, we still need to change the function name here. Maybe get_pin_hash()?

thomaszwagerman commented 2 years ago

Hi @juliasilge, thank you for you reply, and no worries at all for the wait!

Apologies, I was not aware of the unexported pin_hash() - I've implemented your suggestion by changing the function name to get_pin_hash().

github-actions[bot] commented 2 years ago

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.