add global option to specify default dpi

[jan-glx]

fixes #20

[evanbiederstedt]

Thanks @jan-glx

We'll review this.

If nothing else, you've pointed out there some typos in the roxygen2 docs (which is a leftover from the major rewrite we did a few months ago). I only noticed this now.

We should also do a first major release for CRAN, e.g. version 1.0.0

CC @VPetukhov

[evanbiederstedt]

Hi @jan-glx

Thanks for the PR and issue.

Could you write a few sentences regarding the rationale behind this change? I'm guessing you would like to globally set this parameter for all plots, correct?

[jan-glx]

That's right! I use rasterize a lot and don't want to add the , dpi every time to switch between 600 dpi for production and less for development.
Global options are generally considered evil for good reason but for human intended output formatting that can be quite useful but should still not be set in reusable code.
A good example for using options in a similar scenario is given by this commit in data.table

On a side note: I think you should soft-deprecate the old geom_..._rast functions before 1.0 / CRAN

[evanbiederstedt]

On a side note: I think you should soft-deprecate the old geom_..._rast functions before 1.0 / CRAN

I see where you're coming from. We've considered this, but they're too widely used. So we've simply created interfaces using these old function names to the new setup. Some users have been confused that certain parameters are no longer used, but they appear to be the minority. We've only heard back from one person, tbh.

Throwing a warning at this stage after several months feels a bit confusing/pedantic. I worry it will cause more confusion than necessary.

[VPetukhov]

Hi @jan-glx , thanks for the PR!

@evanbiederstedt , configuring defaults seems totally reasonable to me. My only question here is whether we expect to have more in the future. Because if we do, I think that the data.table approach is really inconvenient: you never remember all the names of the options and need to go to the source code for that. Both ggplot2 and knitr store these options in special environments with setters and getters. I think that having a setter with a documented list of options as parameters would be of huge help for users if there is more than one option.

But if we think that it's only dpi we want to configure, I think the current solution is fine. Maybe, we only need to set the default in the .onLoad as in data.table, so we don't need to write 'getOption("ggrastr.default.dpi", 300)' in every single function.

[jan-glx]

I did consider setting ggrastr.default.dpi in .onLoad but decided against it since you would need to have separate options for the old functions (default 300) and the new general approach (default NULL). NULL seems highly desirable as it would defer choosing a reasonable dpi to the graphics device ( which potentially could make better choice), but not desirable enough to change the default and potentially partially break people's code. (and getOption("ggrastr.default.dpi") == getOption("ggrastr.default.dpi", NULL)

[VPetukhov]

NULL seems highly desirable as it would defer choosing a reasonable dpi to the graphics device

Indeed, that makes a lot of sense, thanks!

[evanbiederstedt]

Hi @VPetukhov

But if we think that it's only dpi we want to configure, I think the current solution is fine.

I would be in favor of this approach.

EDIT: That is, only doing this for dpi

[evanbiederstedt]

This does work:

library(ggplot2)
library(ggrastr)
options(ggrastr.default.dpi=750)

plot <- ggplot(diamonds, aes(carat, price, colour = cut))

my_plot = plot + rasterise(geom_point()) + theme(aspect.ratio = 1)
print(my_plot) ## uses dpi 750

[evanbiederstedt]

LGTM

I'll add a comment in the vignettes clarifying things

[evanbiederstedt]

@VPetukhov I updated the vignettes and CHANGELOG in this PR as well. Please have a look.

We can update the version after this to 0.2.2 on the main branch, and update on CRAN.

Thanks, Evan

[evanbiederstedt]

I think this is reasonable. We can have a larger discussion on the PR from dev -> main