search

r-lib / pkgdown

Generate static html documentation for an R package
https://pkgdown.r-lib.org/
Other
713 stars 335 forks source link

build_reference() changes broke something? #1930

Closed Adafede closed 2 years ago

Adafede commented 2 years ago

Hi,

I read the changelogs and it looks like the build_reference() function now errors, without providing an understandable message error.

I re-installed version 1.6.1 and it worked well ``` pkgdown::build_site() -- Installing package into temporary library --------------------------------------------------------------------- == Building pkgdown site ======================================================= Reading from: '/Users/rutza/Git/timaR' Writing to: '/Users/rutza/Git/timaR/docs' -- Initialising site ----------------------------------------------------------- Copying '../../../../Library/Frameworks/R.framework/Versions/4.1/Resources/library/pkgdown/assets/pkgdown.js' to 'pkgdown.js' Writing '404.html' -- Building home --------------------------------------------------------------- Writing 'authors.html' Deprecated: markdown_github. Use gfm instead. Reading '.github/CODE_OF_CONDUCT.md' Writing 'CODE_OF_CONDUCT.html' [WARNING] Deprecated: markdown_github. Use gfm instead. Reading '.github/CONTRIBUTING.md' Writing 'CONTRIBUTING.html' [WARNING] Deprecated: markdown_github. Use gfm instead. Reading '.github/SUPPORT.md' Writing 'SUPPORT.html' [WARNING] Deprecated: markdown_github. Use gfm instead. Reading 'LICENSE.md' Writing 'LICENSE.html' [WARNING] Deprecated: markdown_github. Use gfm instead. -- Building function reference ------------------------------------------------- Writing 'reference/index.html' Reading 'man/biological_cleaning.Rd' Writing 'reference/biological_cleaning.html' Reading 'man/biological_weighting.Rd' Writing 'reference/biological_weighting.html' Reading 'man/chemical_cleaning.Rd' Writing 'reference/chemical_cleaning.html' Reading 'man/chemical_decoration.Rd' Writing 'reference/chemical_decoration.html' Reading 'man/chemical_weighting.Rd' Writing 'reference/chemical_weighting.html' Reading 'man/clean_gnverifier.Rd' Writing 'reference/clean_gnverifier.html' Reading 'man/dist_get.Rd' Writing 'reference/dist_get.html' Reading 'man/dist_groups.Rd' Writing 'reference/dist_groups.html' Reading 'man/export_params.Rd' Writing 'reference/export_params.html' Reading 'man/form_adducts_neg.Rd' Writing 'reference/form_adducts_neg.html' Reading 'man/form_adducts_pos.Rd' Writing 'reference/form_adducts_pos.html' Reading 'man/get_params.Rd' Writing 'reference/get_params.html' Reading 'man/homemadeShift.Rd' Writing 'reference/homemadeShift.html' Reading 'man/log_debug.Rd' Writing 'reference/log_debug.html' Reading 'man/manipulating_taxo_otl.Rd' Writing 'reference/manipulating_taxo_otl.html' Reading 'man/ms1_annotation.Rd' Writing 'reference/ms1_annotation.html' Reading 'man/ms1_decoration.Rd' Writing 'reference/ms1_decoration.html' Reading 'man/non_ms1_annotation.Rd' Writing 'reference/non_ms1_annotation.html' Reading 'man/parse_cli_params.Rd' Writing 'reference/parse_cli_params.html' Reading 'man/parse_yaml_params.Rd' Writing 'reference/parse_yaml_params.html' Reading 'man/parse_yaml_paths.Rd' Writing 'reference/parse_yaml_paths.html' Reading 'man/plot_histograms.Rd' Writing 'reference/plot_histograms.html' Reading 'man/preclean_gnverifier.Rd' Writing 'reference/preclean_gnverifier.html' Reading 'man/prepare_hierarchy.Rd' Writing 'reference/prepare_hierarchy.html' Reading 'man/prepare_hierarchy_2.Rd' Writing 'reference/prepare_hierarchy_2.html' Reading 'man/prepare_plot.Rd' Writing 'reference/prepare_plot.html' Reading 'man/read_clusters.Rd' Writing 'reference/read_clusters.html' Reading 'man/read_edges.Rd' Writing 'reference/read_edges.html' Reading 'man/read_features.Rd' Writing 'reference/read_features.html' Reading 'man/read_metadata.Rd' Writing 'reference/read_metadata.html' Reading 'man/read_nap.Rd' Writing 'reference/read_nap.html' Reading 'man/read_results.Rd' Writing 'reference/read_results.html' Reading 'man/taxo_decoration.Rd' Writing 'reference/taxo_decoration.html' Reading 'man/tidyeval.Rd' Writing 'reference/tidyeval.html' Reading 'man/timaR-package.Rd' Writing 'reference/timaR-package.html' Reading 'man/y_as_na.Rd' Writing 'reference/y_as_na.html' -- Building articles ----------------------------------------------------------- Writing 'articles/index.html' Reading 'vignettes/hello_world.Rmd' Writing 'articles/hello_world.html' -- Building news --------------------------------------------------------------- [WARNING] Deprecated: markdown_github. Use gfm instead. Writing 'news/index.html' == DONE ======================================================================== -- Previewing site ----------------------------------------------------------------------------------------------- ```

But when using version 2.0.0:

pkgdown::build_site()
-- Installing package into temporary library ---------------------------------------------------------------------
== Building pkgdown site =======================================================
Reading from: '/Users/rutza/Git/timaR'
Writing to:   '/Users/rutza/Git/timaR/docs'
-- Initialising site -----------------------------------------------------------
Copying '../../../../Library/Frameworks/R.framework/Versions/4.1/Resources/library/pkgdown/BS5/assets/pkgdown.js' to 'pkgdown.js'
-- Building home ---------------------------------------------------------------
Writing 'authors.html'
Reading '.github/CODE_OF_CONDUCT.md'
Writing 'CODE_OF_CONDUCT.html'
Reading '.github/CONTRIBUTING.md'
Writing 'CONTRIBUTING.html'
Reading '.github/SUPPORT.md'
Writing 'SUPPORT.html'
Reading 'LICENSE.md'
Writing 'LICENSE.html'
Writing '404.html'
-- Building function reference -------------------------------------------------
Writing 'reference/index.html'
Reading 'man/biological_cleaning.Rd'
Error : Failed to parse Rd in biological_cleaning.Rd
ℹ subscript out of bounds
Erreur : callr subprocess failed: Failed to parse Rd in biological_cleaning.Rd
ℹ subscript out of bounds
Type .Last.error.trace to see where the error occurred
which then leads to: ``` .Last.error.trace Stack trace: Process 57007: 1. pkgdown::build_site() 2. pkgdown:::build_site_external(pkg = pkg, examples = examples, ... 3. callr::r(function(..., crayon_enabled, crayon_colors, pkgdown_internet) { ... 4. callr:::get_result(output = out, options) 5. throw(newerr, parent = remerr[[2]]) x callr subprocess failed: Failed to parse Rd in biological_cleaning.Rd ℹ subscript out of bounds Process 68716: 17. (function (..., crayon_enabled, crayon_colors, pkgdown_internet) ... 18. pkgdown::build_site(...) 19. pkgdown:::build_site_local(pkg = pkg, examples = examples, run_dont_run = r ... 20. pkgdown:::build_reference(pkg, lazy = lazy, examples = examples, ... 21. purrr::map(topics, build_reference_topic, pkg = pkg, lazy = lazy, ... 22. pkgdown:::.f(.x[[i]], ...) 23. base:::withCallingHandlers(data_reference_topic(topic, pkg, examples_env = ... 24. pkgdown:::data_reference_topic(topic, pkg, examples_env = examples_env, ... 25. magrittr:::`%>%`(sections %>% purrr::map(as_data), purrr::map(add_slug)) 26. purrr::map(., add_slug) 27. purrr::map(., as_data) 28. pkgdown:::.f(.x[[i]], ...) 29. pkgdown:::as_data.tag_value(.x[[i]], ...) 30. pkgdown:::describe_contents(x, ...) 31. purrr::map(pieces, parse_piece) 32. pkgdown:::.f(.x[[i]], ...) 33. base:::.handleSimpleError(function (err) ... 34. pkgdown:::h(simpleError(msg, call)) 35. rlang:::abort(msg, parent = err) 36. rlang:::signal_abort(cnd) 37. base:::signalCondition(cnd) 38. (function (e) ... x Failed to parse Rd in biological_cleaning.Rd ℹ subscript out of bounds ```

Any hints?

maelle commented 2 years ago

Looking at https://github.com/taxonomicallyinformedannotation/tima-r, I have two questions (not necessarily implying these are the causes of the error!):

#' @param annotationTableWeightedBio

should be

#' @param annotationTableWeightedBio description of the parameter blabla
Adafede commented 2 years ago

Hi,

Thank you for your reply and for having a look directly at the repo! :)

I actually removed them for the website to build (see https://github.com/taxonomicallyinformedannotation/tima-r/commit/fa29fa242994d3280e169d6772f4c777e5c6748e)

When regenerating them locally with devtools, the issue persists.

Yup, nothing is documented for now...

maelle commented 2 years ago

So, it seems pkgdown assumes the docs have to be complete :wink:

If I create a package (usethis::create_package(), usethis::use_r("test")) with R/test.R that is

#' Title
#'
#' @param a 
#'
#' @return 
#' @export
#'
#' @examples
test <- function(a) {
  a
}

and then document() and then build the website, I get the error.

If I make the script

#' Title
#'
#' @param a description
#'
#' @return something
#' @export
#'
#' @examples
test <- function(a) {
  a
}

then document then build the website, things are fine.

So at the current state yes you first need to fill the roxygen2 tags (or add @noRd for now which will give an R CMD check warning).

Adafede commented 2 years ago

Oh wow, this is new then :sweat_smile:

Guess I'll try documenting and coming back in case!

Thank you very much for your help!

GegznaV commented 2 years ago

Similar issue with 2.0.0:

Error : .l must be a list, not NULL

Works fine with pkgdown 1.6.1.

The last time I got a similar issue after package updates was when yaml options structure changed, but no sensible defaults were created to account for the changes between different versions of the package. I'm not sure if this is the case this time.

If my issue is not related to this thread, I can open a new one.

Details (I'm trying to build https://github.com/r-hyperspec/hyperSpec after building and documenting it in RStudio):

> pkgdown:::build_site_external(lazy = TRUE)
== Building pkgdown site =======================================================
Reading from: 'D:/r-hyperspec/hyperSpec'
Writing to:   'D:/r-hyperspec/hyperSpec/docs'
-- Initialising site -----------------------------------------------------------
-- Building home ---------------------------------------------------------------
Reading 'CONTRIBUTING.md'
Writing '404.html'
-- Building function reference -------------------------------------------------
Error : `.l` must be a list, not NULL
Error: callr subprocess failed: `.l` must be a list, not NULL
Type .Last.error.trace to see where the error occurred
> .Last.error.trace

 Stack trace:

 Process 15020:
 1. pkgdown:::build_site_external(lazy = TRUE)
 2. callr::r(function(..., crayon_enabled, crayon_colors, pkgdown_internet) { ...
 3. callr:::get_result(output = out, options)
 4. throw(newerr, parent = remerr[[2]])

 x callr subprocess failed: `.l` must be a list, not NULL 

 Process 27180:
 16. (function (..., crayon_enabled, crayon_colors, pkgdown_internet)  ...
 17. pkgdown::build_site(...)
 18. pkgdown:::build_site_local(pkg = pkg, examples = examples, run_dont_run = r ...
 19. pkgdown:::build_reference(pkg, lazy = lazy, examples = examples,  ...
 20. pkgdown:::build_reference_index(pkg)
 21. pkgdown:::render_page(pkg, "reference-index", data = data_reference_index(p ...
 22. pkgdown:::render_page_html(pkg, name = name, data = data, depth = depth)
 23. utils::modifyList(data_template(pkg, depth = depth), data)
 24. base:::stopifnot(is.list(x), is.list(val))
 25. pkgdown:::data_reference_index(pkg)
 26. magrittr:::`%>%`(meta %>% purrr::imap(data_reference_index_rows,  ...
 27. base:::unlist(., recursive = FALSE)
 28. purrr::compact(.)
 29. purrr:::discard(.x, function(x) is_empty(.f(x)))
 30. purrr:::probe(.x, .p, ...)
 31. purrr:::map_lgl(.x, .p, ...)
 32. purrr::imap(., data_reference_index_rows, pkg = pkg)
 33. purrr:::map2(.x, vec_index(.x), .f, ...)
 34. pkgdown:::.f(.x[[i]], .y[[i]], ...)
 35. purrr::transpose(contents)
 36. purrr:::stop_bad_type(NULL, "a list", what = NULL, arg = ".l")
 37. rlang:::abort(message, x = x, expected = expected, actual = actual,  ...
 38. rlang:::signal_abort(cnd)
 39. base:::signalCondition(cnd)
 40. (function (e)  ...

 x `.l` must be a list, not NULL
GegznaV commented 2 years ago

So at the current state yes you first need to fill the roxygen2 tags (or add @noRd for now which will give an R CMD check warning).

As a user, I expect that the package pkgdown should work fine out of the box after the update if it did fine with the previous version. This update and changed behavior may break CI/CD workflow on, e.g., GitHub Actions only due to the changes which result in an error with no informative error message that describes what really causes the issue.

My suggestion is:

1) either to fix the new version of pkgdown so that it will not break anything (preferred), e.g., by using sensible defaults; 2) or create informative error messages or warnings that reflect the essence what is the real cause of the issue.

(By the way, I really appreciate your efforts to bring the package to the next level.)

Adafede commented 2 years ago

I can confirm filling the skeletons with TODO fixed the issue. Regarding the comment above I would say in between: I think it is the right way to go not to accept empty documentation anymore, but it should be stated (maybe also mentioned in the changelogs). as long as a TODO does the trick for the lazy of us

hadley commented 2 years ago

@GegznaV I don't see any evidence that your issue is the same as this one. I'd suggest you create a new issue with reprex, and avoid pontificating on your expectations of open source software that you get for free.

hadley commented 2 years ago

We will obviously fix this; @maelle has provided you with a work around in the interim.

hadley commented 2 years ago

It's the empty @returns that's the problem, not the empty @params