[R-pkg-devel] Extending/adding to an R6 class from another package: qns

[Hadley Wickham]

> AzureRMR: the "base" package, provides a number of R6 classes
> AzureVM: a "child" package that extends classes from AzureRMR with extra functionality related to virtual machines
> AzureStor: another child package that extends classes from AzureRMR, this time for storage accounts
> Etc.
>
> For example, AzureRMR defines a class called "az_resource_group" that represents an Azure resource group. Within this class, I have convenience functions to manage individual Azure resources: az_resource_group$get_resource(), az_resource_group$create_resource(), etc. One benefit of this approach is that method chaining works: I can do something like
>
>    az_subscription("xxx")$get_resource_group("yyy")$get_resource("zzz").
>
> In my child packages, I then define further classes and methods for dealing with specific services. For consistency, I also add convenience functions to the base AzureRMR::az_resource_group class to work with these new classes. For example, AzureVM defines a new class az_vm_template, and also adds a $get_vm() method to AzureRMR::az_resource_group.
>
> Running devtools::check() however brings up a note and warning for the child packages. For example, with AzureVM:
>
> * checking R code for possible problems ... NOTE
> File 'AzureVM/R/add_methods.R':
>   .onLoad calls:
>     message("Creating resource group '", resource_group, "'")
>
> Package startup functions should use 'packageStartupMessage' to  generate messages.
> See section 'Good practice' in '?.onAttach'.
>
> . . .
>
> * checking for code/documentation mismatches ... WARNING
> Functions or methods with usage in documentation object 'get_vm' but not in code:
>   get_vm get_vm_cluster list_vms
>
>
> The reason for the note is because modifying R6 classes from another package has to be done at runtime, ie, in the .onLoad function. The message() call referred to is inside one of the new methods that I define for an AzureRMR class, hence it never actually appears at package startup. I assume it's okay to ignore this note?

I think monkey-patching classes on load is an extremely bad idea. You
would be better off subclassing, or if the classes are so closely
inter-related, you should put them in a single package. Or re-design
your interface to use the pipe instead of method chaining so this
isn't a problem (brief discussion at
https://adv-r.hadley.nz/oo-tradeoffs.html#tradeoffs-pipe)

> The reason for the warning is because writing documentation for R6 methods is rather awkward, even/especially with Roxygen. This goes doubly so when the method in question is for a class from a different package. What I've done is to write a Roxygen block for the method as if it was a standalone function; for example, the documentation for az_resource_group$get_vm() is like this:
>
> #' Get existing virtual machine(s)
> #'
> #' Method for the [AzureRMR::az_subscription] and [AzureRMR::az_resource_group] classes.
> #'
> #' @rdname get_vm
> #' @name get_vm
> #' @usage
> #' get_vm(name)
> #' get_vm(name, resource_group = name)
> #'
> #' @param name The name of the VM or cluster.
> #' @param resource_group For the `az_subscription` method, the resource group in which `get_vm()` will look for the VM. Defaults to the VM name.
> #'
> #' @details
> #' ...
> NULL
>
> This way, typing ?get_vm will bring up the relevant page, which seems to me to be the best compromise in terms of the end-user experience. Is this an acceptable way of doing the documentation for CRAN?

I think the usage should be consistent with how people actually call
the function, i.e. x$get_vm(name). I'm not sure if R CMD check will
like this, but I suspect it will silence the warning.

Hadley

-- 
http://hadley.nz