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

glue .transformer support (particularly for length-0 and NULL values) #51

Open
mmuurr opened this issue Sep 14, 2021 · 2 comments
Open

Comments

@mmuurr
Copy link
Contributor

mmuurr commented Sep 14, 2021

Since {glue} is vectorized, the default behavior when an interpolated value is length-0 (which includes NULL) is to return character(0):

str(glue::glue("foo is {NULL}")
# 'glue' chr(0)

This causes problems with the msg disappearing for LoggerGlue log events if the glue template includes a value that evaluates to length-0 or NULL.

With {glue}, one can use a transformer to convert these annoyances into something useful:

null0_transformer <- function(text, envir) {
  out <- glue::identity_transformer(text, envir)
  if (is.null(out)) {
    "NULL"
  } else if (length(out) == 0) {
    sprintf("%s(0)", mode(out))
  } else {
    out
  }
}

glue::glue("foo is {NULL}", .transformer = null0_transformer)
# foo is NULL

How about a .transformer optional arg (with glue::identity_transformer as the default) for the logging methods in a LoggerGlue, forwarding on those values to glue::glue()?

Perhaps the same for the .na optional arg in glue::glue()?

(I'm also happy making these changes in a PR if you think it's a good idea but are otherwise busy.)

@mmuurr
Copy link
Contributor Author

mmuurr commented Sep 14, 2021

Ah, I see now that the do.call() dispatching to glue::glue() already can pass along additional named args (e.g. .transformer), but perhaps it'd make sense to set options like these as part of the Logger initialization (i.e. get_logger_glue())?

@s-fleck
Copy link
Owner

s-fleck commented Sep 14, 2021

Hmm handling NULL and zero length vectors is definitely something that should be addressed... This problem applies to normal loggers as well as logger glue... Whether or not I want logger glue to have a default transformer or not is something I still have to think about. Logger config is already quite complex and I don't want to add to many features; at some point "power users" are just better of creating their own subclasses.

Regardless of that, when displaying variable values I recommend doing this as custom fields, eg:

theuser <- NULL
lg$info("signing in user", user = theuser)

This will lead to logs that are more machine-readable/parsable.

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

2 participants