Support multiple pages

[vandenman]

An attempt to fix #84 and allow support for multiple pages. I've added the argument onefile to enable/ disable multiple pages (disabled by default for backward compatibility).

A minimal example:

dir <- "~/svglite_svgs" # some directory where you want to make svgs
file0 <- file.path(dir, "Rplot%03d.svg")

svglite::svglite(file0) # onefile = TRUE
ggplot2::qplot(iris$Species)
ggplot2::qplot(iris$Sepal.Length)
dev.off()

# only one file made
file.exists(file0)

svglite::svglite(file0, onefile = FALSE)
ggplot2::qplot(iris$Species)
ggplot2::qplot(iris$Sepal.Length)
dev.off()

file1 <- sprintf(file0, 1)
file2 <- sprintf(file0, 2)

# two files made
file.exists(file1)
file.exists(file2)

The snippet above also works with svgstring, which now returns a vector where each element represents one svg.

If multiple pages are disabled and a new one is opened, it just pastes the new plot on top. That's not very elegant, but I think other devices do the same. In addition, I've added:

• unit tests to assert that svgs are the same regardless of whether they're created in a single page or from multiple.
• documentation for the onefile argument.

Comments and suggestions are welcome!

[thomasp85]

I'd rather have a breaking change in terms of default file naming than add the complexity of the one-file switch. Are you up for updating this PR for that?

Also, even if it is just as a suggested package, I'd rather avoid taking on ggplot2 as a dependency just for this. can you update your tests to use base/grid graphic instructions?

[vandenman]

I'd rather have a breaking change in terms of default file naming than add the complexity of the one-file switch.

Just so I understand properly, you prefer to remove the argument onefile, and instead always create a single file per call to svglite(...)?

Are you up for updating this PR for that?

Of course!

Also, even if it is just as a suggested package, I'd rather avoid taking on ggplot2 as a dependency just for this. can you update your tests to use base/grid graphic instructions?

Sure, I'll change the test.

[thomasp85]

Yes - I'd rather have it always work as potential multiple output. I think consistency across the different devices is worth the breaking change.

[vandenman]

Ok, so just like e.g., grDevices::svg, I'll make the default behavior onefile=FALSE. Do you want me to remove the argument onefile completely? I can imagine some use cases for onefile=TRUE.

[thomasp85]

I'd rather just remove it altogether... what use (aside from backwards compatibility) do you envision?

[vandenman]

The use case mainly concerns robustness and flexibility. Unfortunately, not all packages are equally careful when opening a new graphics device. For example:

# cran versions
library(svglite)
library(abtest)
# example from abtest::plot_posterior
data <- list(y1 = 10, n1 = 28, y2 = 14, n2 = 26)
ab <- ab_test(data = data, posterior = TRUE)
svglite(tempfile())
plot_posterior(x = ab, what = "logor") # crashes
dev.off()

This example should only create one figure but the function opens an additional graphics device while creating the figure and thus svglite complains. You can see this as a bug in the abtest package but also as a lack of flexibility in svglite, as all base R graphics devices do not throw an error. If you remove onefile, the code above would always create 2 svg files, one that is empty and one that contains a figure.

Another way to handle this is to only create multiple figures if the filename contains e.g., %03d.

[thomasp85]

It should def only create multiple files if the filename supports it, otherwise it should simply overwrite the existing file. Basically I'd like it to implement the same logic as ragg
(https://github.com/r-lib/ragg/blob/b9920e61316693b47d46cbd850e587b4fdf4918d/src/AggDevice.h#L192-L217)

The C code for converting the input string to the appropriate file name is here https://github.com/r-lib/ragg/blob/b9920e61316693b47d46cbd850e587b4fdf4918d/src/AggDevicePng.h#L24

[vandenman]

Changes in the last commit:

• removed onefile.
• removed ggplot2 from suggests.
• Implemented logic of ragg for converting the input string to the appropriate file name.
• Like ragg, if a new device is opened the old plot is overwritten.

@thomasp85 Further comments or feedback are welcome!

[thomasp85]

I'd like to avoid copying code from base R as it limits which license we can use in the future. Is there any reason for this check in general? is there a failure with certain file names?

[vandenman]

I did some tests with snprintf and you get a segmentation fault if you use "%s.svg" (and perhaps with others, I didn't check all patterns). Something like %f does work but for the file "one %f two %d .svg" you get one 0.000000 two 1 .svg. That's not a crash but I also cannot imagine any use case for multiple patterns or for non-integer patterns.

I'd like to avoid copying code from base R as it limits which license we can use in the future.

If you prefer I can also rewrite this check. I copied the code to ensure consistency with base R.

[thomasp85]

I see — thanks for checking that. I’d prefer to have something rewritten rather than copied as it makes licensing easier

[vandenman]

Okay so I refactored checkIntFormat to invalid_filename. I'm not a legal expert on whether the function is sufficiently different, but at least it looks a bit different now.

[thomasp85]

This is great. Thanks! As a last point, can you add an entry in the news file?

[thomasp85]

Thank you!