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

Does str_c no longer coerce non-characters to character as of 1.5.0? #510

Closed
D3SL opened this issue May 22, 2023 · 4 comments
Closed

Does str_c no longer coerce non-characters to character as of 1.5.0? #510

D3SL opened this issue May 22, 2023 · 4 comments
Labels
reprex needs a minimal reproducible example

Comments

@D3SL
Copy link

D3SL commented May 22, 2023

I've been using str_c as part of a shortcut to dump tryCatch errors into text popups in Shiny for years now and after updating to 1.5.0 it appears that's broken now. Other packages depending on Stringr such as SPATA2 also appear to have been impacted by this.

The thing is I don't remember any console warnings of an upcoming breaking change, it's a significant break with expected behaviour based on the other major paste-alikes, and I can't find any mention of this in the function reference. The release notes for 1.5.0 do vaguely mention "many more arguments" but don't confirm what was involved and as mentioned the manpage doesn't mention this stringent requirement.

Was this one of the breaking changes? And if so is there any chance of this being temporarily downgraded back to a warning with the notification that str_c will be breaking significantly from traditional expected paste-like behavior in the near future?

@hadley
Copy link
Member

hadley commented Aug 4, 2023

Do you have a reprex? str_c() appears to work fine with plenty of non-character vectors:

library(stringr)

str_c(1:3)
#> [1] "1" "2" "3"
str_c(TRUE, FALSE, TRUE)
#> [1] "TRUEFALSETRUE"
str_c(1 + 2i, "x")
#> [1] "1+2ix"

Created on 2023-08-04 with reprex v2.0.2

This is already more generous that what is documented, which specifies that the elements of ... must be character vectors.

@hadley hadley added the reprex needs a minimal reproducible example label Aug 4, 2023
@D3SL
Copy link
Author

D3SL commented Aug 5, 2023

I did a bit of testing after your reply and it looks like it's not non-character vectors writ large, but a subset of them. My use case was with TryCatch(), which is trivial to reproduce, I'm not sure what SPATA2 was doing internally.

tryCatch({
    length(foo)
},error=function(e){
    str_c("this will error",e)
})
> rlang::last_trace(drop = FALSE)
<error/vctrs_error_scalar_type>
Error in `str_c()`:
! `..2` must be a vector, not a <simpleError/error/condition> object.
---
Backtrace:1. ├─base::tryCatch(...)
 2. │ └─base (local) tryCatchList(expr, classes, parentenv, handlers)
 3. │   └─base (local) tryCatchOne(expr, names, parentenv, handlers[[1L]])
 4. │     └─value[[3L]](cond)
 5. │       └─stringr::str_c("error", e)
 6. │         └─vctrs::vec_size_common(!!!dots)
 7. └─vctrs:::stop_scalar_type(`<fn>`(`<smplErrr>`), "..2", `<env>`)
 8.   └─vctrs:::stop_vctrs(...)
 9.     └─rlang::abort(message, class = c(class, "vctrs_error"), ..., call = call)

@hadley
Copy link
Member

hadley commented Aug 5, 2023

Ah ok, as the message states, str_c() only works with vector inputs. I think unfortunately you just happened to accidentally rely on a bug in the previous implementation of str_c(); there was never any intent you should be able to parse in any R object and magically get a string. If you want this code to continue to work you can use as.character(), or specific to this case, conditionMessage().

@hadley hadley closed this as completed Aug 5, 2023
@D3SL
Copy link
Author

D3SL commented Aug 6, 2023

It is unfortunate. All of the major paste-alikes follow the same pattern: Concatenate strings as-is, and coerce non-strings (more or less) to be a string as-printed. This doesn't just work for vectors like numerics, logicals, and datetimes, but also pretty much anything a reasonable user would think could be treated as text.

str_c()'s new behavior doesn't just break with that, it's also internally inconsistent. Leaving vectors of non-strings aside str_c() will still accept non-vectors like dataframes and lists (including ragged ones). tryCatch()'s e is a list consisting of a character vector and a call.

I get that I'm arguing with the de facto king of R here but I urge you to consider that even the developers of a significantly advanced R package, power users who know the inner workings of R and metaprogramming, were blindsided and stumped by this.

Objectively what happened here was an unannounced break with language-wide established behavior, that isn't documented, that isn't consistent, and that breaks other things downstream puzzling even very experienced developers.

If rolling back and giving a deprecation warning isn't an option I think at minimum the documentation should be updated to make it clear str_c() is non-idiomatic and accepts only vectors and list-like collections of vectors as of X version.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
reprex needs a minimal reproducible example
Projects
None yet
Development

No branches or pull requests

2 participants