ropensci / dev_guide

rOpenSci Packages: Development, Maintenance, and Peer Review
https://devguide.ropensci.org
Creative Commons Attribution Share Alike 4.0 International
159 stars 54 forks source link

Silencing warnings in 4.6.2 Reviewers #806

Closed thomaszwagerman closed 5 months ago

thomaszwagerman commented 5 months ago

Hi there,

I was just having a read of the 4.6.2 Reviewers section and noticed the following warnings above the reviewer list output:

## Warning in value[[jj]][ri] <- if (is.factor(xij)) as.vector(xij) else xij:
## number of items to replace is not a multiple of replacement length

The reviewer author list is generated by the following code:

#| echo: false
#| results: 'asis'
#| eval: !expr has_airtable_access
editors <- c(
  "Noam Ross", "Karthik Ram", "Maëlle Salmon",
  "Anna Krystalli", "Mauro Lepore", 
  "Laura DeCicco", "Julia Gustavsen",
  "Emily Riederer", "Adam Sparks", "Jeff Hollister"
  )
reviewers <- airtabler::airtable(base = "app8dssb6a7PG6Vwj", 
                                table = "reviewers-prod")
reviewers <- reviewers$`reviewers-prod`$select_all()
reviewers <- reviewers[purrr::map_lgl(reviewers$reviews, 
                               ~!is.null(.)) & 
                         !(reviewers$name %in% c(editors, "???")), ]
# get last names
last_names <- humaniformat::last_name(trimws(reviewers$name))
reviewers <- reviewers[order(last_names), ]
reviewers$name[is.na(reviewers$name)] <- reviewers$github[is.na(reviewers$name)]
cat(paste0("[", reviewers$name, "](https://github.com/", reviewers$github, ")", collapse = " \U00B7 "))

I do not have access to the AIRTABLE_API_KEY (nor should I), so I cannot test further to suggest a tidier change to the code itself by running it. I guess there is a mismatch in the number of author names / GitHub links in the airtable?

I've added a #| warning: false output option to prevent it happening in the future, as I'm guessing that might be a common occurrence as author details change.

I am happy to contribute further changes if needed. I'm also happy to open an issue instead of this PR, to be addressed by an editor (with access to the API key) if that suits you better. Perhaps keeping warnings is desirable because they have flagged an error that may otherwise have gone unnoticed, but I will leave that up to you :)!

mpadge commented 5 months ago

Thanks @thomaszwagerman. I approved the PR just to build and check. It builds at https://devdevguide.netlify.app/softwarereview_intro#editors-and-reviewers, where you can see for yourself that, unfortunately, the problem has not yet been solved. I'm confident @maelle will have further ideas here ...

maelle commented 5 months ago

Thank you @thomaszwagerman!!