-
Notifications
You must be signed in to change notification settings - Fork 33
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
Use ragg if available #138
base: main
Are you sure you want to change the base?
Conversation
stop("Unknown entry in getOption('repr.plot.backends'): ", o) | ||
) | ||
stop( | ||
"No plotting devide found in getOption('repr.plot.backends'): ", |
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.
Type: devide should be device.
Since this is an message intended for end users: maybe it could be even more explicit and say something like
stop(
"None of the backends specified in getOption('repr.plot.backends'): ",
toString(backends):
"seems to be available."
"\n Did you forget ..."
"Please specify a valid backend using 'options(repr.plot.backends = c(
)
(or, instead of available, you could say 'working' or whatever you think is the most accurate).
png(tf, width, height, 'in', pointsize, bg, res, antialias = antialias) | ||
|
||
dev.cb <- function(tf) get_device( | ||
ragg = ragg::agg_png(tf, width, height, 'in', pointsize, bg, res), # scaling, bitsize |
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.
Looking at the code, at first I would have expected this to cause issues if the ragg
package was not present on the end-user system, since I'd expect the namespace to not be known. Previously, for Cairo, you had it wrapped in if-else statemtents, so then the Cairo namespace would clearly only be called if the is_cairo_installed()
returned true.
However, I initially didn't have ragg, and this line didn't produce any issues. I'm no R expert, so just want to check: are you sure this will work ok, even if some of the backends are not installed? And which function now imports this namespace? Before, I believe that was done by the requireNamespace
call that was done by e.g. is_cairo_installed()
, but now, it's not clear to me.
Fixes IRkernel/IRkernel#604 … maybe