fix as_label()

[kennedymwavu]

this is how as_label() is currently defined:

#' Make label
#' 
#' Cheap replacement for rlang::as_label to avoid dependency.
#' Must fix.
#' 
#' @noRd
#' @keywords internal
as_label <- function(x) {
  name <- tryCatch(
    is.character(x),
    error = function(e) e
  )

  if(!inherits(name, "error"))
    return(x)

  deparse(substitute(x, parent.frame()))
}

i'm a bit lost regarding how this function is supposed to work or what the expected output is. maybe we should add examples?

nevertheless, i believe it has a bug:

as_label(something) # "x"
as_label(hello) # "x"
as_label(x + y) # "x"

could it be that the intended results were these?

as_label(something) # "something"
as_label(hello) # "hello"
as_label(x + y) # "x + y"

[JohnCoene]

Can't remember if it is still used or where but looking at the function it is meant to be called from a parent function.

foo <- \(){
  as_label("hello")
  y <- 1
  as_label(y)
}()

I haven't tested this yet

[kennedymwavu]

here are the occurrences it's used in:

• ambiorix/R/response.R

  Line 341 in 35aab44

  name <- as_label(name)

• ambiorix/R/response.R

  Line 356 in 35aab44

  name <- as_label(name)

• ambiorix/R/response.R

  Line 365 in 35aab44

  name <- as_label(name)

• ambiorix/R/response.R

  Line 557 in 35aab44

  name <- as_label(name)

if you look at all those occurrences, it seems that the intended use was for as_label() to return the same result for both of these calls, but that's not what happens:

f <- function(a) {
  as_label(a)
}

f("hello") # "hello"
f(hello) # "x"

but if that's not the case, then as_label() has no real use in all those occurrences and can be omitted altogether.
i could be missing something though, so i'd appreciate your input on this @JohnCoene

[JohnCoene]

Yes, would have to be.

f <- function(x) {
      as_label(x)
}

f(hello)
f("hello")

My knowledge of metaprogramming in R is still limited, is this something we should keep anyways? What do you think?

[kennedymwavu]

for our use-case, no, as_label() isn't necessary. if you look at all those occurrences, you'll notice that the variable name is almost always expected to be a string.

here's how you'd expect, say set(), to be used:

res$set(name = "xyz", value = 10)

rarely will someone do:

res$set(name = xyz, value = 10)

unless they've defined xyz in scope. eg.

xyz <- "xyz"
res$set(name = xyz, value = 10)

that's why i say as_label() isn't needed.

but assuming you'd expect someone to do res$set(name = xyz, value = 10) with no definition of xyz in scope, then this is the most likely definition & usage of as_label() you'd want to have:

#' Make label
#'
#' Cheap replacement for rlang::as_label to avoid dependency.
#' Must fix.
#'
#' @noRd
#' @keywords internal
as_label <- function(x) {
  call <- match.call()
  if (is.character(call[["x"]])) {
    return(x)
  }

  deparse(substitute(x))
}

as_label(a) # "a"
as_label("b") # "b"
as_label(c) # "c"

f <- function(a) {
  call <- match.call()

  # pass value of 'a' as is:
  do.call(what = "as_label", args = list(x = call[["a"]]))
}

f("hello") # "hello"
f(hello) # "hello"
f("something") # "something"

# if evaluation of value is needed:
x <- 10
do.call(what = "f", args = list(a = x)) # "10"

but as far as i can tell, we do not need as_label().

[JohnCoene]

Thanks for sharing. I agree with you it's probably better to remove it