Reinforce that skip is due to a package
Have observed this with some packages that might not easily be detected as such, e.g.
skip_if_not_installed("parameters")
skip_if_not_installed("stats")
Alternatively, we could quote-wrap the name to make it clear the package is not a normal noun.
Incidentally fixing #1967
Maybe use the {packagename} convention?
Maybe use the {packagename} convention?
Sure, this is pretty identical to the reasoning now laid out in the style guide: https://style.tidyverse.org/documentation.html#package-names
If the package name might be misinterpreted as an ordinary word, disambiguate by following it with “package” or by wrapping the package name in {} (but not both).
Using { makes it less tempting to use paste() 😉
(happy to update the other tests if we're aligned on the core fix)
Yeah, I think so.
Although looking at again, I wonder if "cannot be loaded" should be "is not installed"? That's technically less correct, but I think almost every time a package fails to load is because it's not installed.
I would keep the more technically correct wording, I have run into a few cases where requireNamespace() failed on an installed package and it can be very confusing to untangle if you're thinking "but it's installed!".
A more involved approach could be to surface the error message:
found <- tryCatch(find.package(pkg), error=identity)
if (inherits(found, "error")) skip("Not installed")
else if (!requireNamespace(pkg, quietly=TRUE)) {
skip("Failed to load")
}
Actually I think find.package() offers a nice way through here, added it in the latest commit.
I like it!
OK, great! I will tidy this up with passing tests later today or tomorrow.
For #1967, I think the current change is already enough. Just the non-ASCII leading character { should be enough to wind up grouping these messages together, and it's probably enough to just have skip() messages citing packages all grouped together (i.e., even if a non-skip_if_not_installed() skip message uses {pkg} to start, it's good to have that grouped with the skip_if_not_installed() messages; the possibility a skip() message starts with { and is not related to a package name is ignored as very unlikely.
Fine with me.
Thanks for wrapping up! Slipped my mind 😵💫
No problems!