Question: Why does `xgx_scale_x_log10()` return a list with a scale instead of the scale directly?

[billdenney]

Why does xgx_scale_x_log10() return a list with a scale instead of the scale directly?

In the code below (and I'd guess in other places, too), a list is returned on success (if there is not a "try-error"), and a ggplot2 scale is returned directly if there is a failure. It seems like it'd be preferable to always have the same output type rather than varying based on different inputs.

https://github.com/Novartis/xgxr/blob/40cbe15aa55c097de6e421ac75ea8783ad04b996/R/xgx_scale_x_log10.R#L38-L40

[iamstein]

Hey @margoal1 and @mattfidler , do you remember why we did this? I can't remember if we had a reason for doing it this way, or if this was just kind of a random quirk we introduced.

[margoal1]

@iamstein I have no idea. Didn't you mainly work on this function? I don't know why it would need the "list". I guess it could be tested without the "list" part and see under what conditions it fails, if any.

[margoal1]

@iamstein xgx_scale_x_time_units also returns a list, but that one seems to change the xlab as well as the scale. The other xgx_scale functions return a scale directly.

[iamstein]

Ooh, I looked back at the oldest version of this function I could find, back before we even had xgxr, and back then, we were always returning a list because we were changing the tick marks. We stopped doing this, but I guess kept the list. So I do think it could make sense to remove the list now, just for consistency. I'd propose to address this issue along with some others when we make the next package update in CRAN (perhaps at the time we add the other functions @billdenney proposed).

xgx_scale_x_log10 = function(decade.spacing=1,x1=-10,x2=10,color="grey50",...) {
  if (length(intersect(c("breaks","labels"),names(list(...))))==0) {
    scale_command = scale_x_log10(breaks=log.breaks(decade.spacing,x1,x2),
                                  labels=log.labels(decade.spacing,x1,x2),
                                  ...)
  } else {    
    scale_command = scale_x_log10(...)
  }
  return(list(
    scale_command,
    annotation_logticks(base = 10, sides = "b", color = "grey50"))
  )
}

[billdenney]

Yeah, if you're doing two things (returning two ggproto objects), then a list is needed.

[iamstein]

Updated to return ggproto instead of list.