-
Notifications
You must be signed in to change notification settings - Fork 2k
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
file extension parameter for ggsave() to allow device specification for files without extension #939
Conversation
What advantage does this approach have over passing this information as the
|
Hi Brian,
will save a png with tiny width and height because the default units are "px" and ggsave passes the parameters in inches. Instead, a workaround requires specifying these parameters (
This works just fine and I'm using this approach myself but it's not very intuitive. I'm happy to keep using this workaround but wanted to suggest the above solution in case you guys would be interested in a more intuitive implementation. Another way of doing it without introducing any extra parameters would be by allowing
which would make this work directly:
What do you think? |
Thanks for the explanation; you are right that passing the device function misses the |
|
Looks good. Can you please do the last two things listed above? |
Thanks Hadley, can you clarify real quick if you prefer the implementation discussed with Brian above allowing I'm happy to implement the latter and file as a separate pull request (+NEWS) or stick with the current and include it in NEWS. Let me know, thanks. |
Ok, yeah, I'd prefer |
Sounds good, I implemented it with Update: the Travis CI build is failing but it appears to be because of a syntax error elsewhere, not in this pull request (see #949 for reference) |
I'm not really sure how to resubmit this pull request for Travis CI, but it should build fine now. #949 fixed the error. |
Yeah, I didn't know how to do that (resubmit the request for Travis), it builds just fine for me with #949 fixed. I'm happy to resubmit but the only way I can think of doing that is closing this pull request and creating a new one - is that the recommended approach? |
According to this: http://stackoverflow.com/q/17606874/892313 it should be possible to trigger a new attempt at the build. But maybe Hadley (or one of the repo owners) has to do it because I don't see the triggering icon there. |
Just a quick follow-up, I think this feature could still be very useful to users judging from the traffic that the original post on stack overflow still receives (save-plots-made-in-a-shiny-app). As far as I can tell, all that's missing is a retriggering of the Travis CI build here by a repo owner. @hadley could you help out with this? Alternatively, I'm happy to close this pull request and create a new one for this feature if that's easier, let me know. |
if(!exists(ext, mode = "function")) { | ||
stop("No graphics device defined for the file extension '", ext, "'. ", | ||
"Make sure to specify a filename with supported extension or ", | ||
"set the device parameter.") |
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.
Can you please add call. = FALSE
here?
Just update this PR by merging/rebasing against master, and re-pushing the branch to github. |
Are you still interested in this PR? |
Parameter `device` now supports character argument to specify which supported device to use ('pdf', 'png', 'jpeg', etc.), for when it cannot be correctly inferred from the file extension (for example when a temporay filename is supplied server side in shiny apps)
Okay, should be all ready to go now (including the requested .call = FALSE addition and some commit cleanup). Thanks for the reminder msg, missed last week's while traveling. And thanks a lot for the rebasing hint to retrigger Travis CI build, that's exactly what I was missing - learned something new again. |
file extension parameter for ggsave() to allow device specification for files without extension
Thanks! |
This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/ |
Thank you for the fantastic ggplots package!
This is a suggested implementation for specifying a file extension in
ggsave
to allow device matching (pdf, png, jpg, etc.) for filenames without extension. The suggestion is motivated by the problem of using ggsave in a shinyApp for letting the user download a plot (for example addressed on StackOverflow here: http://stackoverflow.com/questions/14810409/save-plots-made-in-a-shiny-app). Because the shiny server provides a temporary file without extension, ggsave can't match to any of the internally defined device functions. I'm not aware of a currently available solution to this problem and although there are several work-arounds, I like the simplicity of ggsave and wanted to suggest this solution (not sure if it's the best way to do it, but it's the most elegant I could think of). This would allow use ofggsave
for a shinyAppdownloadHandler
like so:I'm relatively new to GitHub and this is my second contribution to someone else's project, please forgive if I'm missing some specific conventions or etiquette about forking/pull requests.
Sebastian