-
Notifications
You must be signed in to change notification settings - Fork 13
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
Lower DPI for vignette images to fix CRAN errors and notes #323
Conversation
In my testing, replacing the |
The actual error in vignette building is probably just related to sharpshootR, not the image compression, which should be resolved by #322. Regardless it would be a good idea to get the package below 5MB per CRAN guidelines. |
OK Thanks. I'm fine with the lower-res images for now, maybe there is a way to preserve the higher quality stuff on the website. That is where most people (I think) will be reading these articles. On a related note, I'm going to move MASS into imports, as I've got additional reasons to use that package in the future. |
Agreed. My approach to this would be to set an environment variable in the gh-pages build action, e.g. |
So, MASS depends on R 4.4.0+... so if you import it you will need to bump the minimum R version aqp relies on. I am not sure that is a good idea. Since it appears the only actual usage of it is in the rarely-used Notwithstanding future plans, I think at this point it is a tall order to require R 4.4.0 for soilDB and packages that suggest it... |
I implemented the environment variable solution for DPI in a476432 |
Nice idea for the variable res output. I think that dropping MASS may be the last TODO before trying another CRAN submission. Waiting for windbuilder to report back on the latest changes. |
It seems that #322 is not sufficient to clear the biggest errors in the CRAN checks related to large image sizes. The most recent submission was rejected. While "noSuggests" is certainly a significant additional issue, it appears that the total package size due to images is a bigger one, triggering ERROR on six platforms (e.g. https://www.r-project.org/nosvn/R.check/r-devel-linux-x86_64-debian-clang/aqp-00check.html)
CRAN check gives the following hard errors when building vignettes. The output is indicating that with better compression or image output options, the PNG images could be reduced in size by ~20 - 30%.
I have never seen this check trigger before, and can find no information or instances of it occurring on Google other than in the aqp check log. It appears to be targeting the rather large "doc" directory generated in the package after building vignettes. Lowering the DPI in all vignettes to
48
(dots per inch) gets the "doc" directory down well below 1MB compared to the currrent 1.6MB (see #322 (comment))We still exceed the suggested installed size in CRAN repository policy by about 200kB, but the "doc" folder is not flagged in this NOTE
Going to
32
DPI for all vignettes (ad41a98) gets the whole package down below 5 MB and clears the NOTE. There are, of course, other places we could optimize package size, but it seems these images are the target of new scrutiny. IMO they really aren't that important to the overall package function or vignette content, the users can run the code locally to generate full resolution images if they follow the vignette instructions.We should make moves to correct this issue at a minimum to clear the errors and keep the package on CRAN, then we can deal with other areas for optimization in a near-future release.
I suggest that in the interim we implement a much lower DPI on CRAN. The only alternative I see is if we can introduce some sort of compression, or use a different format e.g. JPEG, to make higher DPI images sufficiently small. These options also have drawbacks.
We can make the lowered DPI setting conditional when we build the GitHub Pages site (https://ncss-tech.github.io/aqp/) so that the small/harder-to-read images are only generated in the CRAN vignette builds, whereas our website has full resolution.
One possible alternative option allowing us to keep something like the current settings might be to use the OptiPNG knitr hook: https://bookdown.org/yihui/rmarkdown-cookbook/optipng.html. We would need to check with maintainers to confirm it would work on CRAN, it appears that they are using this tool to run the checks on PNGs that are causing ERRORs. While we could potentially provide local builds of vignettes in lieu of CRAN building them--using this external tool would probably not be workable on CCE devices.