mrkaye97 / slackr

An R package for sending messages from R to Slack
https://matthewrkaye.com/slackr/
Other
307 stars 83 forks source link

allow finer control over image size in slackr_dev #169

Closed shabbychef closed 3 years ago

shabbychef commented 3 years ago

When sending images via slackr_dev, they always appear as square and somewhat low resolution. This can be fixed by passing e.g. width and height values to dev.copy. Perhaps simply passing dots like this would work:

slackr_dev <- function(channels = Sys.getenv("SLACK_CHANNEL"), token = Sys.getenv("SLACK_TOKEN"), 
                       file = "plot", initial_comment = NULL, title = NULL, thread_ts = NULL, ...) {
    ftmp <- tempfile(file, fileext = ".png")
    dev.copy(png, file = ftmp, ...)
    dev.off()
    res <- slackr:::files_upload(file = ftmp, channels = channels, initial_comment = initial_comment, 
        token = token, thread_ts = thread_ts)
    return(invisible(res))
}
mrkaye97 commented 3 years ago

@shabbychef I'm going to label this one as wontfix in favor of using some other method of uploading plots that I'll reconsider this / next week. My main issue with slackr_dev is that the nature of copying the current device somehow and then uploading it to Slack as an image feels a little bit too automagical to me (FWIW, I feel the same about ggslackr). It's hard to test and is much less predictable than making you provide a plot you want to upload, which I think I prefer.

I will rework this function so that you can do something like:

plot <- hist(iris$Sepal.Length)

## maybe will extend slackr_upload?
slackr_upload(
  plot,
  ext = ".png"
)

^ that will not work now, since slackr_upload serializes whatever you want to upload, but I think it's a reasonable path forward. Let me know if you have more thoughts on this -- happy to discuss!

shabbychef commented 3 years ago

I'm fine with that. The issue here is that a plot object has to become a file to be uploaded, which may require setting height, width and so on.