Suppress (or attend to) warnings and messages in vignettes

[IndrajeetPatil]

Follow-up on #338

The check-vignette-warnings workflow should pass.

You can work on this issue either when #338 is resolved for the repo(s) you are maintaining, or work on both issues at the same time.

Preamble

Same as #338

Examples

Same as #338

Progress tracker

• insight
• modelbased
• bayestestR
• effectsize
• parameters
• performance
• correlation
• report
• see
• datawizard
• easystats

[mattansb]

@IndrajeetPatil Why am I getting an error here? https://github.com/easystats/effectsize/actions/runs/3946604514/jobs/6754520336

I locally ran with options(warn = 2) and didn't get warnings or errors...

[IndrajeetPatil]

You will have to put options(warn = 2) in the setup chunk of the vignette you see failing in CI/CD, and then you can reproduce the error locally.

{knitr} knits the documents in a fresh environment, so it's not affected by what options you have set locally.

[mattansb]

Yes, that's exactly what I did - added in the setup chunk, then knitted... No error.

[etiennebacher]

For easystats/datawizard#354:

• when I manually knit the problematic vignette, it runs fine (even with options(warn = 2) in the setup).
• when I run

options(crayon.enabled = TRUE, warn = 2L)
vignettes <- fs::dir_ls("vignettes/", glob = "*.Rmd", recurse = TRUE)
purrr::walk(vignettes, rmarkdown::render)

then I have the same error as in the workflow

• when I run

options(crayon.enabled = TRUE, warn = 2L)
vignettes <- fs::dir_ls("vignettes/", glob = "*.Rmd", recurse = TRUE)
for (i in vignettes ) rmarkdown::render(i)

it works fine

Is there really a need for purrr::walk(...)?

[strengejacke]

Just curious what would happen if you use lapply() instead of walk()?

[etiennebacher]

Just curious what would happen if you use lapply() instead of walk()?

For me it fails with the same error as walk()

[IndrajeetPatil]

@mattansb I can reproduce the error locally if I do:

options(crayon.enabled = TRUE, warn = 2L)
vignettes <- fs::dir_ls("vignettes/", glob = "*.Rmd", recurse = TRUE)
lapply(vignettes, rmarkdown::render)

[IndrajeetPatil]

For me it fails with the same error as walk()

So this is definitely something systematic and not specific to how walk() works.

[mattansb]

I would say that if we get this warning only when running knitr through an lapply, the check-vignette-warnings action isn't well calibrated. (Also,
I have warnings suppressed in all vignettes where relevant, never had an issue on CRAN, pkgdown, or GHA- why do we need such a strict check?

[IndrajeetPatil]

I know all these checks seem pretty onerous, but here is my thinking.

If we are running into these issues right now, it is only a matter of time before one of our users runs into them, and the probability keeps increasing as the user base grows. lapply() and walk() are not some obscure functions; they are used frequently by most users, so this issue is bound to be encountered by someone at some point.

Aren’t you a tiny bit curious under what circumstances this issue might be occurring?

[mattansb]

In my specific case, the issue arises due to some scoping problem that is only of concern when knitting this particular vignette under a specific set of conditions; I don't see any end user ever coming up against this.

I agree we should know why our functions are returning warnings, but (1) as discussed elsewhere, sometimes we want users to see (in examples, in vignettes) that a function will warn when used in a certain way (not just write in the docs "the function will give a warning when..."), and (2) if, as in my case, the warning only occurs under an obscure set of conditions, it is of no relevance.

So:

1. Dealing with warnings in vignettes and examples by setting verbose = FALSE is counter productive - I don't want my users thinking that this is how to deal with warnings!
2. Perhaps Suppress (or attend to) warnings and messages in vignettes #348 and Suppress (or attend to) warnings and messages in examples #343 can be a lintr like GHA, where it "marks" where these warnings happen, and we can voluntarily choose if and how to deal with them on a case-by-case basis?

[IndrajeetPatil]

sometimes we want users to see (in examples, in vignettes) that a function will warn when used in a certain way (not just write in the docs "the function will give a warning when...")

Dealing with warnings in vignettes and examples by setting verbose = FALSE is counter productive - I don't want my users thinking that this is how to deal with warnings!

I disagree with the premise behind both of these points.
Users should always see happy path examples, not corner cases where the function manages to recover with a warning. Those should be included only in the test suite.

Perhaps #348 and #343 can be a lintr like GHA, where it "marks" where these warnings happen, and we can voluntarily choose if and how to deal with them on a case-by-case basis?

This won't work because we are finding warnings by turning them into errors. Otherwise, they are going to go unnoticed, just like messages. Currently, we have no way to find and suppress messages. We just need to go example-by-example and check which ones are producing messages and suppress them. Turning warnings into errors helps us to immediately discover the source of those warnings.

as in my case, the warning only occurs under an obscure set of conditions, it is of no relevance

In that case, I would recommend dropping this workflow for {effectsize}.

I have tried to clarify my thoughts in my slides on when and why some of these workflows are helpful. Whatever workflow doesn't work for you because you don't share the same goal I had in mind, you can just drop that workflow.

[mattansb]

I disagree with the premise behind both of these points.
Users should always see happy path examples, not corner cases where the function manages to recover with a warning. Those should be included only in the test suite.

I disagree with your disagreement! 😅

1. For obscure warnings, yes. But if there are common use cases that warrant a warning/message, having only "happy path" examples is a disservice to our users, because...
2. Users have a FoF response to messages/warnings (let's no even speak of errors!) - having an example that shows when a warning happens, what it means, and how to deal with it, is PRICELSS!
   We can decide that such things will only appear in vignettes and not in examples, by ignoring sure cases when they are common is a recipe for verbose = FALSE...

Suggested new function:

test_verbose <- function(verbose) {
  if (!isTRUE(verbose)) {
    insight::format_warning("You have chosen to blind yourself to any inconvenient truths by setting 'verbose = FALSE'. BEWARE!")  
  }
  invisible()
}

[IndrajeetPatil]

Users have a FoF response to messages/warnings (let's no even speak of errors!) - having an example that shows when a warning happens, what it means, and how to deal with it, is PRICELSS!

I think the crux of the issue is if you have a situation that users might run into often and that your function anticipates, then it really shouldn't be a warning, it should be a message. Because, otherwise, warnings lose their potency. But this is a very subjective decision and I would trust your (author/developer) judgment on deciding what is a corner case worth a warning, and what is an anticipated case worth a message.

[IndrajeetPatil]

having an example that shows when a warning happens, what it means, and how to deal with it, is PRICELSS!

By that logic, we should also be showing the errors, because if warnings cause panic, imagine what error messages do to them.

Also, you are assuming here that users are looking at the executed examples in pkgdown website. That's suspicious. More likely, they are looking at the examples in help pages or PDF manual, where they have no way of telling which example generates a warning, and which one doesn't.

[IndrajeetPatil]

Here is a concrete example of what I mean.

I see no value in having an example like this in the documentation. The thought here is only to show that if the user were to mis-specify an argument, the function would still work. They will find this out on their own if and when they make the said mistake. Having this in examples makes it sounds like this is a common occurrence, when it's supposed to be an exception (and thus we are emitting a warning, and not a message).

library(datawizard)

winsorize(rnorm(10), threshold = 2)
#> Warning: `threshold` for winsorization must be a scalar between 0 and 0.5.
#>   Did not winsorize data.
#>  [1]  0.35008438  0.01351593 -0.82309322  0.41848684  0.47729241 -0.83810507
#>  [7] -1.04797084 -0.35694537 -0.40491704  1.18079812

^{Created on 2023-01-21 with reprex v2.0.2}

[mattansb]

I think the crux of the issue is if you have a situation that users might run into often and that your function anticipates, then it really shouldn't be a warning, it should be a message.

I think there is an issue here stemming from our dual task of being good software devs and being good statistical guides. If there is a statistical analysis that is commonly performed under non-ideal conditions, the software dev would say "I was able to easily anticipate the software being used in this way, so this is a non issue", but the stats guide would say "this statistical procedure was not meant to be used in this way, but we were able to 'recover', so let's give the user a warning".

Having this in examples makes it sounds like this is a common occurrence, when it's supposed to be an exception.

Thus, I think you are mistaken in equating "warning-worthy" with "rare" 👆 - something can statistically be very common (😭) but still worthy of a warning.

So having an example that shows when a stats-related warning happens, what it means, and how to deal with it, is PRICELSS, and is not the same as an example where the user input the wrong kind of object and so got a warning.

Also, you are assuming here that users are looking at the executed examples in pkgdown website.

No - we should have commented examples where code will produce a warning.

[IndrajeetPatil]

Okay, I get the confusion now.

Apparently, we have been speaking two different languages: me talking about the "warning" as a component of R's condition system, while you using it as a flag for statistical advice to the user.

But if that's what you have in mind, then technically you don't need to use a warning condition. You can also use a message to warn users about something they might be doing that's (statistically speaking) a bad idea. I guess this is exactly what you are now doing in your PR by going from format_warning -> format_alert.