fix: Remove deprecation message from upstream polars in some cases with %in%#259
Conversation
|
I don't think this would change any behavior. The message is about a deprecated usage that will change in a future version of I don't have a good way to test the absence of a message since it appears that this is not really considered as a standard message by R, i.e. those tests wouldn't work as expected before this PR: expect_message(
iris_pd |>
head() |>
filter(Species %in% c("setosa", "virginica"))
)
# Error (but shouldn't):
# ! `filter(head(iris_pd), Species %in% c("setosa", "virginica"))` did not throw the expected message.
expect_silent(
iris_pd |>
head() |>
filter(Species %in% c("setosa", "virginica"))
)
# doesn't error while it shouldSo I just added an extra test to ensure that the Thanks for opening the issue and the PR! |
polars in some cases with %in%
|
As far as I can tell, the deprecation message seems to go through the OS-level stderr. A workaround for testing whether testthat::test_that("`is_in` deprecation warning is not captured", {
scriptToRun <-
'sink <- dplyr::filter(tidypolars::as_polars_df(head(iris)), Species %in% "setosa")'
result <- system2(
R.home("bin/Rscript"), c("-e", shQuote(scriptToRun)), stderr = TRUE
)
testthat::expect_true(!any(grepl("issues\\/22149", result)))
}) |
|
Thanks for the workaround, but that seems quite hacky and wouldn't work on Windows as far as I can see so I'm not convinced it should be part of the test suite. I had seen the deprecation message before, I just couldn't have it reliably so let it aside until you reported it. |
|
Just wondering, doesn't snapshot testing work? I think r-polars tests messages generated on the Rust side with snapshot tests. |
|
True, forgot about that, I guess I was mostly annoyed by the fact this doesn't happen on Windows (and potentially macOS, I can't test on this platform). I'll add a test later |
|
I just tested and the message also doesn't appear in snapshot tests (before the fix). |
|
polars' handle_result bridge doesn't seem to handle Rust-side warnings, so the message bypasses R and cannot be intercepted by R-side sinks. Whether or not the message appears is dependent on how the R console/process captures the subprocesses' stderr. On my Windows machine, the message does not appear in RGui or RStudio, but does appear if using Rscript or Emacs+ESS. |
|
Oh, thanks for the detail. |
Presumably fixes (#258); not sure if this change has implications elsewhere in the codebase.