easystats / datawizard

Magic potions to clean and transform your data 🧙
https://easystats.github.io/datawizard/
Other
208 stars 14 forks source link

Fix build failures on macOS 4.3 #527

Closed IndrajeetPatil closed 1 week ago

IndrajeetPatil commented 1 month ago

cf. https://cloud.r-project.org/web/checks/check_results_datawizard.html

Version: 0.12.1
Check: running R code from vignettes
Result: ERROR 
  Errors in running code in vignettes:
  when running code in ‘tidyverse_translation.Rmd’
    ...
  > if (!all(vapply(pkgs, requireNamespace, quietly = TRUE, 
  +     FUN.VALUE = logical(1))) || getRversion() < "4.1.0") {
  +     evaluate_chunk <- FALSE
   .... [TRUNCATED] 

  > row("Note: In this vignette, we use the native pipe-operator, `|>`, which was introduced in R 4.1. Users of R version 3.6 or 4.0 should replace the  ..." ... [TRUNCATED] 

    When sourcing ‘tidyverse_translation.R’:
  Error: a matrix-like object is required as argument to 'row'
  Execution halted

    ‘selection_syntax.Rmd’ using ‘UTF-8’... [1s/1s] OK
    ‘standardize_data.Rmd’ using ‘UTF-8’... [4s/5s] OK
    ‘tidyverse_translation.Rmd’ using ‘UTF-8’... failed
Flavor: [r-oldrel-macos-arm64](https://www.r-project.org/nosvn/R.check/r-oldrel-macos-arm64/datawizard-00check.html)

As discussed elsewhere, a solution might just be to rename the row() function to not collide with base::row().

etiennebacher commented 1 month ago

I don't think this will solve it. If base::row() is called it means that evaluate_chunk was FALSE, so even if we rename our row() to something like custom_note(), there will be an error because custom_note() won't be found.

It's still a mystery why this fails only on macOS with R 4.3, but I think one solution would be to get rid of the custom function and use pure HTML, i.e. replace

```{r echo=FALSE}
row("Note: In this vignette, we use the native pipe-operator, `|>`, which was introduced in R 4.1. Users of R version 3.6 or 4.0 should replace the native pipe by magrittr's one (`%>%`) so that examples work.")
by

Note: In this vignette, we use the native pipe-operator, `|>`, which was introduced in R 4.1. Users of R version 3.6 or 4.0 should replace the native pipe by magrittr's one (`%>%`) so that examples work.


That way, this note will be displayed, no matter if `htmltools` is available or not.
IndrajeetPatil commented 1 month ago

I can't even reproduce the CRAN behaviour locally on macOS on R 4.3.

I am happy with just using raw HTML to avoid this issue.

strengejacke commented 1 month ago

And why isn't this failing in the other vignette, from which I copied the code?

etiennebacher commented 1 month ago

We don't load tidyr in the other, that might be the cause ?

IndrajeetPatil commented 1 month ago

This is still not fixed 😭, even with 0.12.2 release: https://www.r-project.org/nosvn/R.check/r-oldrel-macos-x86_64/datawizard-00check.html

checking running R code from vignettes ... [9s/17s] ERROR
Errors in running code in vignettes:
when running code in ‘tidyverse_translation.Rmd’
  ...
+     FUN.VALUE = logical(1))) || getRversion() < "4.1.0") {
+     evaluate_chunk <- FALSE
 .... [TRUNCATED] 

> data_filter(starwars, skin_color == "light", eye_color == 
+     "brown")

  When sourcing ‘tidyverse_translation.R’:
Error: could not find function "data_filter"
Execution halted

Let's spend some time trying to reproduce this locally and then fix it. Otherwise, the problems are going to continue to compound.

strengejacke commented 1 month ago

That's more than strange, since data_filter() is a datawizard function. I'd say, let's move the vignettes to online only, and just refer to the overview on CRAN, like we do for parameters for instance: https://cran.r-project.org/web/packages/parameters/index.html

strengejacke commented 1 month ago

I think one reason can be that we do not use evaluate_chunk in every chunk. Hence, there are some chunks that are evaluated, even if they possibly shouldn't?

# since we explicitely put eval = TRUE for some chunks, we can't rely on
# knitr::opts_chunk$set(eval = FALSE) at the beginning of the script. So we make
# a logical that is FALSE only if deps are not installed (cf easystats/easystats#317)
evaluate_chunk <- TRUE

if (!all(vapply(pkgs, requireNamespace, quietly = TRUE, FUN.VALUE = logical(1L))) || getRversion() < "4.1.0") {
  evaluate_chunk <- FALSE
}
strengejacke commented 1 month ago

That must be the reason. We have:

{r, eval = evaluate_chunk}
library(dplyr)
library(tidyr)
library(datawizard)

data(efc)
efc <- head(efc)

but next chunk:

{r filter, class.source = "datawizard"}
# ---------- datawizard -----------
starwars |>
  data_filter(
    skin_color == "light",
    eye_color == "brown"
  )
...

So if one of the other packages is not available, evaluate_chunk = FALSE, no packages are loaded, and consequently, data_filter() is not found. This is the behaviour that is different from the other vignette, and that's probably why we got a failure for row().

etiennebacher commented 1 month ago

I'll make a PR to add the eval=evaluate_chunk where it's missing.

etiennebacher commented 1 month ago

That must be the reason

Actually I don't think this is the reason. In the "tidyverse translation" vignette, the eval option is set to FALSE at the top. The reason is that we display the datawizard code and the tidyverse code in two code blocks (side by side) and only evaluate the datawizard one afterwards using eval = evaluate_chunk (and there is no place where this is missing).

So basically, it's not possible that a code chunk evaluates without evaluate_chunk being TRUE. Still, R CMD check reports this error:

checking running R code from vignettes ... [9s/17s] ERROR Errors in running code in vignettes: when running code in ‘tidyverse_translation.Rmd’ ...

  • FUN.VALUE = logical(1))) || getRversion() < "4.1.0") {
  • evaluate_chunk <- FALSE .... [TRUNCATED]

data_filter(starwars, skin_color == "light", eye_color == "brown")

When sourcing ‘tidyverse_translation.R’: Error: could not find function "data_filter" Execution halted

I really don't understand why this errors. The only "fix" I can see is to remove the vignettes from the package and keep them online only (as @strengejacke suggested). @IndrajeetPatil, thoughts?

strengejacke commented 2 weeks ago

bump @IndrajeetPatil - maybe we should plan to submit a new fix by the end of August/early September?

etiennebacher commented 2 weeks ago

How does it work to remove vignettes from the package while keeping them on the website? Just put the vignettes folder in .Rbuildignore?

strengejacke commented 2 weeks ago

Yes, that should work. For some packages, I have just left an "overview" vignette, see:

https://github.com/easystats/parameters/blob/main/.Rbuildignore https://github.com/easystats/parameters/blob/main/vignettes/overview_of_vignettes.Rmd https://cran.r-project.org/web/packages/parameters/index.html

etiennebacher commented 3 days ago

928ypf