Skip to content
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

do not eval vignette chunks which should error (use purl=FALSE) #6220

Merged
merged 7 commits into from
Jul 8, 2024

Conversation

ben-schwen
Copy link
Member

@ben-schwen ben-schwen commented Jul 4, 2024

Currently our CI pipelines are erroring, since they error on parts of the vignettes where they are supposed to error.

According to knitr doc setting error=0 should be the right

0 will keep evaluating after errors as if you had pasted the text into a terminal;

but these will still result in R CMD check erroring, hence, purl=FALSE

@ben-schwen ben-schwen changed the title set error=0 in vignette chunks which should error set eval=FALSE in vignette chunks which should error Jul 4, 2024
@jangorecki
Copy link
Member

Afaiu this will result into different output on the vignette. Error won't be printed. Please add error message below the call that mean to error, prefixing with #

@tdhock
Copy link
Member

tdhock commented Jul 4, 2024

actually it would be better to use purl=TRUE as below, which includes the error output in the rendered vignette, but does not cause an error when building the vignette.

```{r error=TRUE, purl=TRUE}
mlr3::ResamplingCV$new()$instantiate(reg.task)
```

example: search for "error:" in https://cloud.r-project.org/web/packages/mlr3resampling/vignettes/ResamplingSameOtherSizesCV.html

@ben-schwen ben-schwen changed the title set eval=FALSE in vignette chunks which should error set purl=TRUE in vignette chunks which should error Jul 4, 2024
@ben-schwen
Copy link
Member Author

ben-schwen commented Jul 4, 2024

@tdhock Ty for the suggestion. Knitted vignette knitr::knit("vignettes/datatable-programming.Rmd") looks good! But I just tried it with the docker image and R CMD check still errors.

@ben-schwen
Copy link
Member Author

ben-schwen commented Jul 4, 2024

Using eval=FALSE and adding the output as comment seems the way to go as suggested by Jan

@tdhock
Copy link
Member

tdhock commented Jul 5, 2024

??? what is the error you see with the docker image ???

@ben-schwen
Copy link
Member Author

ben-schwen commented Jul 5, 2024

??? what is the error you see with the docker image ???

With error = TRUE, purl = TRUE R CMD check is failing with the same error

 ERROR
Errors in running code in vignettes:
when running code in ‘datatable-programming.Rmd’
  ...

> my_subset = function(data, col, val) {
+     subset(data, col == val)
+ }

> my_subset(iris, Species, "setosa")

  When sourcing ‘datatable-programming.R’:
Error: object 'Species' not found
Execution halted

* checking re-building of vignette outputs ... OK
* checking PDF version of manual ... OK
* DONE

Status: 1 ERROR, 2 NOTEs

@ben-schwen ben-schwen changed the title set purl=TRUE in vignette chunks which should error do not eval vignette chunks which should error Jul 5, 2024
@tdhock
Copy link
Member

tdhock commented Jul 7, 2024

this is a new error in my other packages as well (which use purl=TRUE) so I'm thinking this may be a bug in knitr. I checked their issues and found yihui/knitr#2338 where I asked for a work-around.

@tdhock
Copy link
Member

tdhock commented Jul 7, 2024

sorry for the confusing suggestions, but I think the solution is to use purl=FALSE instead of TRUE in chunk options.
(I have several CRAN packages that are not failing checks which use purl=FALSE)

@ben-schwen
Copy link
Member Author

@tdhock I just checked with the docker image. error=TRUE, purl=FALSE works indeed, ty for the suggestion and clarification!

@ben-schwen ben-schwen changed the title do not eval vignette chunks which should error do not eval vignette chunks which should error (use purl=FALSE) Jul 7, 2024
@MichaelChirico
Copy link
Member

LGTM, but will let Toby approve as I think he's more familiar with the {knitr} details here.

@tdhock tdhock merged commit e292428 into master Jul 8, 2024
3 checks passed
@tdhock
Copy link
Member

tdhock commented Jul 8, 2024

thanks

@ben-schwen ben-schwen deleted the vignette_errors branch July 8, 2024 20:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants