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

When na.rm argument is supplied to stringr::str_flatten(), should it error or show a helpful message? #1540

Open
francisbarton opened this issue Sep 16, 2024 · 0 comments

Comments

@francisbarton
Copy link

francisbarton commented Sep 16, 2024

stringr::str_flatten() is translated to STRING_AGG(), which (on MSSQL) removes NULLs by default.
Therefore the na.rm argument has been excluded from the translation.

Two problems:

  1. On local data frames, na.rm = FALSE is the default, whereas from a DB we are effectively getting na.rm = TRUE behaviour by default. (It would need some reworking to make the dbplyr translation behave the same way as the standard function - and I am not suggesting it would be a good idea to do this.) Potentially an issue I could raise on the stringr repository as a documentation suggestion for str_flatten.
  2. If you include na.rm in your function call it throws an error, as it is only expecting the string and collapse arguments. This is understandable. But I suggest instead of throwing an error it could more gracefully ignore the na.rm, displaying an informative warning or info message.

If the developer team are happy with my suggestion in 2., and it makes sense, then I can try to write a patch that displays a message for the user and does not error.

library(dbplyr)
library(stringr)

translate_sql(str_flatten(x), con = simulate_mssql())
#> <SQL> STRING_AGG(`x`, '') OVER ()
translate_sql(str_flatten(x, na.rm = TRUE), con = simulate_mssql())
#> Error in str_flatten(x, na.rm = TRUE): unused argument (na.rm = TRUE)
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

No branches or pull requests

1 participant