Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Use expect_no_warning() #1248

Merged
merged 1 commit into from
Dec 9, 2024
Merged

Use expect_no_warning() #1248

merged 1 commit into from
Dec 9, 2024

Conversation

IndrajeetPatil
Copy link
Collaborator

@IndrajeetPatil IndrajeetPatil commented Dec 9, 2024

Instead of expect_warning() + NA combo.

Automated via:

replace_multiline <- function(dir = "tests/") {
  files <- list.files(dir, pattern = "\\.R$", full.names = TRUE, recursive = TRUE)
  
  pattern <- "(?s)expect_warning\\((.*?)\\,\\s*NA\\)"
  replacement <- "expect_no_warning(\\1)"
  
  for (f in files) {
    txt <- xfun::read_utf8(f)
    content <- paste(txt, collapse = "\n")
    new_content <- gsub(pattern, replacement, content, perl = TRUE)
    
    if (!identical(content, new_content)) {
      new_txt <- unlist(strsplit(new_content, "\n", fixed = TRUE))
      xfun::write_utf8(new_txt, f)
      message("Modified: ", f)
    }
  }
  
  invisible(NULL)
}

replace_multiline("tests/")

Instead of `expect_warning()` + `NA` combo
Copy link
Contributor

github-actions bot commented Dec 9, 2024

This is how benchmark results would change (along with a 95% confidence interval in relative change) if 0d4cc51 is merged into main:

  • ✔️cache_applying: 165ms -> 165ms [-3.08%, +2.47%]
  • ✔️cache_recording: 559ms -> 559ms [-1.25%, +1.23%]
  • ✔️without_cache: 1s -> 997ms [-0.86%, +0.05%]

Further explanation regarding interpretation and methodology can be found in the documentation.

Copy link
Collaborator

@lorenzwalthert lorenzwalthert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you do it yourself or instruct chatGPT or similar to do this? 😄

@IndrajeetPatil
Copy link
Collaborator Author

I started with a regex that didn't work and ChatGPT took it from there 😈

@IndrajeetPatil IndrajeetPatil merged commit 4e3bc30 into main Dec 9, 2024
18 checks passed
@IndrajeetPatil IndrajeetPatil deleted the use-expect-no-warning branch December 9, 2024 09:40
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants