8

So I'm working on an R package that use S3 classes, and it would be really nice if I could use sample as method for one of my classes. However, base already declares sample as a non-S3 function, so what I wonder is:

Is it bad style to redefine a non-S3 base function such as sample as an S3 function? And could this mess things up for users of my package?

A way you could redefine sample and still keep the base function working is:

sample.default <- base::sample
sample <- function(x, ...) {
  UseMethod("sample")
}
# This allows me to define a new sample method for my_special_class
sample.my_special_class <- function(...) {...}

But what I'm uncertain about is whether this will cause any trouble or namespace issues, for example, when loading other packages. I've also noticed that not many packages redefine sample, for example, dplyr uses sample_n and igraph uses sample_, and I thought that there might be some reason for this...

Rasmus Bååth
  • 4,827
  • 4
  • 29
  • 29
  • If you don't have yet, perhaps you could come up with your own class and define a `sample.yourclassname` to handle your classes. In this way you keep the consistency of using generic functions. – Roman Luštrik Feb 14 '17 at 21:12
  • @RomanLuštrik: That doesn't work because `base::sample` isn't generic. If it were generic, there's no reason to re-define it in your package; you would just register your method. – Joshua Ulrich Feb 14 '17 at 21:51
  • @JoshuaUlrich bah, you beat me to it. Just checked the functions seconds before your post. You are correct. Back to the drawing board. – Roman Luštrik Feb 14 '17 at 21:52
  • See http://stackoverflow.com/questions/41884629/turning-an-r-s3-ordinary-function-into-a-generic/41885909#41885909 – G. Grothendieck Feb 14 '17 at 22:14

1 Answers1

9

Whether or not it's bad "style" is primarily opinion-based. But Writing R Extensions, Section 7.1 - Adding new generics tells you how to add new generics that mask base base/recommended functions. That said, your proposed solution is explicitly cautioned against in that section:

...a package can take over a function in the base package and make it generic by something like

 foo <- function(object, ...) UseMethod("foo")
 foo.default <- function(object, ...) base::foo(object)

Earlier versions of this manual suggested assigning foo.default <- base::foo. This is not a good idea, as it captures the base function at the time of installation and it might be changed as R is patched or updated.

One problem others may encounter is if another package also registers summary as a generic. Then packages depending on your package or the other package need to decide which generic to register their method with.

Not many packages redefine sample because it's usually not a good idea to mask base/recommended functions. That creates the potential for users to get different behavior at the top-level depending on whether or not your package is loaded.

One thing you certainly want to avoid is masking a generic in a base/recommended (or highly used) package. Doing so can prevent method dispatch at the top-level, causing headaches for users who expect the generic to work (e.g. dplyr::lag).

Joshua Ulrich
  • 173,410
  • 32
  • 338
  • 418
  • If I understand the manual correctly, CRAN is suggesting that you define `sample.default <- function(x, ...) base::sample(x, ...)` and considers this permissible. – Benjamin Feb 14 '17 at 21:23
  • @Benjamin why not just make up your own class and extend the generic to handle the class as well? – Roman Luštrik Feb 14 '17 at 21:42
  • @RomanLuštrik I believe that's what Rasmus Baath is attempting to do. The question he is posing is what to do with `base::sample` when he redefines `sample <- function(x, ...) UseMethod("sample")` (please correct me if I'm misunderstanding something) – Benjamin Feb 14 '17 at 21:50
  • 1
    @RomanLuštrik by the Law of Large Numbers, it had to happen eventually. :) – Benjamin Feb 14 '17 at 21:54
  • 1
    @Benjamin my life strategy. – Roman Luštrik Feb 14 '17 at 22:09
  • Thanks for the link to Writing R Extensions! So since they mention the possibility my take is that it isn't necessary bad style, but could be problematic because it masks generic `sample` in other packages. – Rasmus Bååth Feb 15 '17 at 07:26
  • 1
    @RasmusBååth: I mentioned that as a possible issue, but it's a fairly small probability. A package would need to use *both* packages that create the generic *and* want to register a `sample` method. – Joshua Ulrich Feb 15 '17 at 14:15