-
Notifications
You must be signed in to change notification settings - Fork 337
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
Version 2.1.0 triggers an error in deploy_to_branch() traced to pkgdown:::is_syntactic() #2727
Comments
Thanks for the detailed exploration! Without thinking the problem through at all, but just thinking about the structure of R calls, I suspect it should be something like this: is_call(x) && !is_call(x, "=") && (is_name(x[[1]]) && !is_syntactic(x[[1]])) |
Hi, No problem, I'm glad that you found it useful =) Your tentative solution does work for the smallest reproducible case I reported. Note that I think that you meant needs_tweak <- function(x) {
rlang::is_call(x) && !rlang::is_call(x, "=") && (rlang::is_named(x[[1]]) && !pkgdown:::is_syntactic(x[[1]]))
}
x <- parse(text = "viridisLite::viridis(21)")[[1]]
needs_tweak(x) devel > needs_tweak <- function(x) {
+ rlang::is_call(x) && !rlang::is_call(x, "=") && (rlang::is_named(x[[1]]) && !pkgdown:::is_syntactic(x[[1]]))
+ }
devel > x <- parse(text = "viridisLite::viridis(21)")[[1]]
devel > needs_tweak(x)
[1] FALSE Workaround for
|
Sorry I meant |
I see, thanks! needs_tweak <- function(x) {
rlang::is_call(x) && !rlang::is_call(x, "=") && (rlang::is_symbol(x[[1]]) && !pkgdown:::is_syntactic(x[[1]]))
}
x <- parse(text = "viridisLite::viridis(21)")[[1]]
needs_tweak(x) devel > needs_tweak <- function(x) {
+ rlang::is_call(x) && !rlang::is_call(x, "=") && (rlang::is_symbol(x[[1]]) && !pkgdown:::is_syntactic(x[[1]]))
+ }
devel > x <- parse(text = "viridisLite::viridis(21)")[[1]]
devel > needs_tweak(x)
[1] FALSE |
The other place you'll see this type of problem is Since you've done the exploration and figured out a solution do you want to close the loop with a PR? |
This commit implements the change @hadley proposed at r-lib#2727 (comment) to the internal function `needs_tweak()`. This does resolve the small reproducible example I described, though I ignore if there are other situations where it wouldn't work for.
I see, thanks. I just submitted the PR (#2728). Thanks again Hadley for providing the solution to the issue! Best, |
Hi,
With
pkgdown
version 2.1.0, I'm running into the following error when usingpkgdown::deploy_to_branch()
.I saw this on GitHub actions at https://github.com/LieberInstitute/spatialLIBD/actions/runs/9944998632/job/27472260912#step:26:256 but I've also reproduced it locally. I've also triggered the same error when I run
pkgdown::build_reference()
directly.Here's my local
deploy_to_branch()
error and R session information.This is happening with https://github.com/LieberInstitute/spatialLIBD and the error message points to https://github.com/LieberInstitute/spatialLIBD/blob/devel/man/vis_gene_p.Rd. If I delete https://github.com/LieberInstitute/spatialLIBD/blob/devel/man/vis_gene_p.Rd manually, then
pkgdown::deploy_to_branch()
does work as shown below.Initial error
Going back to the error, here's the
rlang::last_trace()
output again (it's included in the first details section):The
rlang::last_trace()
output is pointing topkgdown/R/usage.R
Lines 14 to 24 in 2841c0f
Looking at
pkgdown/R/build-reference.R
Lines 280 to 294 in 2841c0f
pkgdown/R/build-reference.R
Lines 161 to 195 in 2841c0f
That leads to:
Further diving into the code of
pkgdown/R/build-reference.R
Lines 328 to 361 in 2841c0f
That leads to:
Small reproducible code
So, as a small piece of reproducible code, here is:
Even smaller reproducible code
Re-running parts of
pkgdown/R/usage.R
Lines 10 to 24 in 2841c0f
I'm able to get to the error again.
Diving further, I see that the issue is element 11 of
parsed
.This 11th element has the
pkg::function(number)
structure. It'sviridisLite::viridis(21)
in particular.I see that it actually leads to a 3 length logical vector output for
pkgdown:::is_syntactic()
.From the above information, the smallest reproducible error I can make is this one:
Solutions?
I know that
pkgdown:::is_syntactic()
is defined atpkgdown/R/utils.R
Line 28 in 2841c0f
So hmm, I have no obvious solutions to propose here. The only one that comes to mind is to change
pkgdown/R/usage.R
Lines 21 to 23 in 2841c0f
any()
tois_syntactic()
as shown below:roxygen2 differences
Having said all the above, I don't understand why https://github.com/LieberInstitute/spatialLIBD/blob/59b0af4f0491bd66f9b04ed8e30c2a0d2aa38d28/man/vis_gene.Rd#L17-L18 doesn't trigger the error since that help file has the exact same documented argument
cont_colors
as https://github.com/LieberInstitute/spatialLIBD/blob/59b0af4f0491bd66f9b04ed8e30c2a0d2aa38d28/man/vis_gene_p.Rd#L16-L21. Maybe it's becauseroxygen2
writes out that argument in different formats on both Rd files. It's the same argument asvis_gene_p()
inherits it fromvis_gene()
https://github.com/LieberInstitute/spatialLIBD/blob/59b0af4f0491bd66f9b04ed8e30c2a0d2aa38d28/R/vis_gene_p.R#L15. So maybe this is not apkgdown
issue but aroxygen2
one?Best,
Leo
The text was updated successfully, but these errors were encountered: