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

Suppress (or attend to) warnings and messages in examples #343

Open
8 of 11 tasks
IndrajeetPatil opened this issue Jan 7, 2023 · 11 comments
Open
8 of 11 tasks

Suppress (or attend to) warnings and messages in examples #343

IndrajeetPatil opened this issue Jan 7, 2023 · 11 comments
Labels
Core Packages 📦 Discussion and planning about core packages of easystats Upkeep 🧹

Comments

@IndrajeetPatil
Copy link
Member

IndrajeetPatil commented Jan 7, 2023

Follow-up on #338

The check-all-examples 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
Copy link
Member

mattansb commented Jan 9, 2023

@IndrajeetPatil Why would this pass but suddenly fail when R-CMD-check-strict was created?

https://github.com/easystats/effectsize/actions/workflows/check-all-examples.yaml

@IndrajeetPatil
Copy link
Member Author

@mattansb It has actually nothing to do with the new strict check.

Previously, the “check all examples” workflow was checking only that all examples run. Now, it additionally checks that examples don’t produce warnings. That’s what the current issue is about.

@mattansb
Copy link
Member

mattansb commented Jan 9, 2023

Ahhhhh ok. Thanks!

@etiennebacher
Copy link
Member

@IndrajeetPatil Is it possible to ignore some warnings (without using suppressWarnings() since we still want to show them to users)? In the following example, a warning is expected and I think it's useful to show the users the potential warnings:

https://github.com/easystats/datawizard/actions/runs/3918766334/jobs/6699343379

library(datawizard)

test <- data.frame(
  a = c("iso", 2, 5),
  b = c("year", 3, 6),
  c = c(NA, 5, 7)
)
test
#>     a    b  c
#> 1 iso year NA
#> 2   2    3  5
#> 3   5    6  7
row_to_colnames(test)
#> Warning: Some values of row 1 were NAs. The corresponding column names are
#>   prefixed with `x`.
#>   iso year x1
#> 2   2    3  5
#> 3   5    6  7

@IndrajeetPatil
Copy link
Member Author

Hmm, I am wondering if it's really necessary to show an example that is going to generate a warning.

Typically, the help page examples will expose the functionality but not the edge cases users might run into, which is where they are likely to see warnings and errors. Does that make sense?

That said, if you do want to retain that example, maybe just set verbose = FALSE?

@etiennebacher
Copy link
Member

Right, it doesn't bring much to show warnings in the example, I'll change the example

@rempsyc
Copy link
Sponsor Member

rempsyc commented Jan 17, 2023

In examples I often see that we load the current package, but I've never seen that anywhere else before. Why do we do that? Should I remove those instances?

@IndrajeetPatil
Copy link
Member Author

Yeah, I have also noticed this. It's definitely not necessary, but I don't feel strongly either way.

I think you can let them be for now. We do have a number of other, more immediate issues to resolve that we can prioritize.

@strengejacke
Copy link
Member

In examples I often see that we load the current package, but I've never seen that anywhere else before. Why do we do that? Should I remove those instances?

Due to lazyness, I copy/paste examples into an R file, and then I don't need to load the related library manually.

@rempsyc
Copy link
Sponsor Member

rempsyc commented Jan 18, 2023

No more warnings in examples for report

@etiennebacher
Copy link
Member

etiennebacher commented Mar 31, 2023

Some examples have warnings (and therefore this check fails) because of the environment problem due to insight:

Could not recover model data from environment. Please make sure your data is available in your workspace. Trying to retrieve data from the model frame now.

This only happens in devtools::run_examples(), not when I run them manually. Should we ignore them? Users shouldn't see these warnings and it might confuse them to add a data <<- data in the examples (if this fixes the issue)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Core Packages 📦 Discussion and planning about core packages of easystats Upkeep 🧹
Projects
None yet
Development

No branches or pull requests

7 participants