-
Notifications
You must be signed in to change notification settings - Fork 318
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
Reinforce that skip is due to a package #1959
Conversation
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
Using |
(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 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 |
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 |
Fine with me. |
Thanks for wrapping up! Slipped my mind 😵💫 |
No problems! |
Have observed this with some packages that might not easily be detected as such, e.g.
Alternatively, we could quote-wrap the name to make it clear the package is not a normal noun.
Incidentally fixing #1967