3

This is tricky because this problem only happens in the context of a package -- everything works as expected when defined in the global namespace.

I've defined an S3 generic called coerce_na_range(), which has two methods, coerce_na_range.factor() and coerce_na_range.default(). coerce_na_range() is exported, but the two methods aren't. (The purpose of the function is to replace numbers encoded as character or factor labels with NA within a given range.)

When defined in the global namespace, if I pass a character vector to coerce_na_range(), it dispatches it to coerce_na_range.default() and works as expected:

vec <- c("green", "yellow", "-9", "red", "-1")
coerce_na_range(vec)
# [1] "green"  "yellow" NA       "red"    NA   

However, if I instead load the package in a fresh session, it seemingly ignores the default method:

library(lighthouse)

vec <- c("green", "yellow", "-9", "red", "-1")
coerce_na_range(vec)
# Error in UseMethod("coerce_na_range") : 
#  no applicable method for 'coerce_na_range' applied to an object of class "character"

I don't think the problem is that the methods aren't exported? e.g., tidyr:::full_seq.Date(), etc., aren't exported, and tidyr::full_seq() obviously works.

The package is hosted at https://github.com/ccsarapas/lighthouse. The code for coerce_na_range(), its methods, and a few functions they depend on, is:

#' Suppress NA warning when coercing to numeric
#'
#' Coerces `x` to numeric. If `x` cannot be coerced, returns `NA` and suppresses
#' coercion warning.
#'
#' @export
try_numeric <- function(x) {
  if (is.factor(x)) {
    warning(
      "`x` is a factor and will be converted based on factor codes, not factor labels."
    )
  }
  withCallingHandlers(
    warning = function(w) {
      if (conditionMessage(w) == "NAs introduced by coercion") {
        rlang::cnd_muffle(w)
      }
    },
    as.numeric(x)
  )
}

#' @rdname try_numeric
#'
#' @export
try.numeric <- function(x) try_numeric(x)

#' Generate NA values of appropriate type
#'
#' Returns compatible `NA` based on `x`. This is usually of the same type as `x`
#' (e.g., `NA_real_` if `x` is a double vector). If `x` is a factor, will
#' return `NA_character_` if `factor_as_character = TRUE` (the default) and
#' `NA_integer_` otherwise.
#'
#' @export
na_like <- function(x, factor_as_character = TRUE, match_length = FALSE) {
  stopifnot("`x` must be an atomic vector" = is.atomic(x))
  type_out <- if (factor_as_character && is.factor(x)) "character" else typeof(x)
  length_out <- if (match_length) length(x) else 1L
  rep(methods::as(NA, type_out), length_out)
}


#' Set NA values based on numbers stored as strings.
#'
#' Changes values coercible to numeric in range `range_min`:`range_max` to `NA`.
#' Useful for imported SPSS files.
#'
#' @export
coerce_na_range <- function(x, ...) UseMethod("coerce_na_range")
coerce_na_range.default <- function(x, range_min = -Inf, range_max = -1) {
  coerced <- try.numeric(x)
  dplyr::if_else(
    is.na(coerced) | (coerced < range_min) | (coerced > range_max),
    x,
    na_like(x)
  )
}
coerce_na_range.factor <- function(x, range_min = -Inf, range_max = -1) {
  lvls <- levels(x)
  coerced <- try.numeric(as.character(lvls))
  lvls <- lvls[is.na(coerced) | coerced < range_min | coerced > range_max]
  factor(x, levels = lvls)
}
zephryl
  • 14,633
  • 3
  • 11
  • 30
  • 1
    I think if you put a `#' @export` line above the `coerce_na_range.factor` function, then Roxygen will identify this as an S3 method, and add `S3method(coerce_na_range, factor)` to your package NAMESPACE file. Same with the default method. – Allan Cameron Nov 21 '22 at 15:49
  • 4
    For `roxygen`, you still need to use `#' @export` on your functions. It's smart enoug to recognize that they are S3 methods and won't export them with their full name, but will set it up so those generic methods can be used by consumers of your package. see https://stackoverflow.com/a/22598245/2372064 – MrFlick Nov 21 '22 at 15:49
  • @AllanCameron so the "real answer" :-) is that the OP needs to understand what's required in the `NAMESPACE` file for things to work as expected. – Carl Witthoft Nov 21 '22 at 15:56
  • or `@exportS3method coerce_na_range factor` in the roxygen code – Stéphane Laurent Nov 21 '22 at 15:58
  • 1
    @StéphaneLaurent Are you sure that's the current recommendation? The help page says that function is "Only needed when the method is for a generic from a suggested package." I'm pretty sure with the latest versions of roxygen you are just supposed to use `@export` and it will do the right thing. See also https://r-pkgs.org/Metadata.html#sec-metadata-namespace – MrFlick Nov 21 '22 at 16:05
  • 1
    @CarlWitthoft we can see that the OP is using Roxygen2, so they shouldn't be adjusting the NAMESPACE manually. Roxygen allows enough abstraction that one can forget about the details of the NAMESPACE file. But knowing how/why it works is always good. – Allan Cameron Nov 21 '22 at 16:10
  • @AllanCameron agreed - I am one of those folks who "trust but verifies" so I would probably double-check the NAMESPACE file after running roxygen – Carl Witthoft Nov 21 '22 at 16:59

0 Answers0