yihui / knitr

A general-purpose tool for dynamic report generation in R
https://yihui.org/knitr/
2.36k stars 873 forks source link

{codetools} is Imports, not Suggests #2277

Closed MichaelChirico closed 10 months ago

MichaelChirico commented 11 months ago

{codetools} is used twice for caching in {knitr}:

https://github.com/yihui/knitr/blob/ec4c9b632e1b35994340848884a31da2ca24640a/R/cache.R#L110-L113

https://github.com/yihui/knitr/blob/ec4c9b632e1b35994340848884a31da2ca24640a/R/block.R#L329-L330

In neither place is proper escaping done for treating it as a suggests (e.g. if (!requireNamespace("codetools", quietly = TRUE)) stop("'codetools' is required for caching")), so I would move the package to Imports.

If this is unintentional / a workaround is warranted, then we could either drop {codetools} or give proper escapes for its usage.

yihui commented 10 months ago

Imports means a heavier dependency, and codetools is required only conditionally, so I prefer not making it a hard dependency. For Windows and macOS users, it's very unlikely that codetools is not installed, so this problem only matters for Linux users (when r-base is installed but not r-base-recommended).

e.g. if (!requireNamespace("codetools", quietly = TRUE)) stop("'codetools' is required for caching")

I agree that the error message is clearer than

Error in loadNamespace(x) : there is no package called ‘codetools’

but this type of escaping is a tedious task for every package in Suggests. I could make it a function and call the function, though, if the current error message looks confusing enough for you. Thanks!

MichaelChirico commented 10 months ago

Closing for now, I'll revisit the next time I see the error. My memory is it came out in a confusing context, but I'll double-check before deciding the right fix.

yihui commented 10 months ago

Thanks! I'm open to enhancing the error message in future.

github-actions[bot] commented 4 months ago

This old thread has been automatically locked. If you think you have found something related to this, please open a new issue by following the issue guide (https://yihui.org/issue/), and link to this old issue if necessary.