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

[Hong Ooi]

I do use subclassing as you suggest. In my base package I have an az_resource class that represents any generic Azure resource. Eg in the VM package, I extend it to obtain az_vm_resource; in the storage package I extend it to obtain az_storage; and so on. In addition to simple subclassing, I also define more complex classes that combine resources of different types.

However, Azure also has a hierarchical structure where a subscription can contain multiple resource groups, each of which can contain multiple resources. So I have a resource_group class that includes functions to manage resources. It's this class that I'm adding methods to at runtime, so that you can work with az_vm_resource objects the same way that you work with az_resource objects.

Given that I'm using R6 (for its persistent state, since I'm tracking Azure resources), and given that I'm writing multiple packages, I don't really see any way around "monkey-patching" classes on load. Azure has approximately 1e6 services on offer, and I don't want to support them via one monster package (in principle).

You can see the work-in-progress on the CloudyR repo if you like:
https://github.com/cloudyr/AzureRMR
https://github.com/cloudyr/AzureVM
https://github.com/cloudyr/AzureStor
https://github.com/cloudyr/AzureContainers

In any case, I've realised I can work around the note about startup messages by simply hiving off all the xxx$set() calls to a secondary function, rather than having them directly in .onLoad().


-----Original Message-----
From: Hadley Wickham <h.wickham using gmail.com> 
Sent: Saturday, 20 October, 2018 2:42 AM
To: Hong Ooi <hongooi using microsoft.com>
Cc: R Package Development <r-package-devel using r-project.org>
Subject: Re: [R-pkg-devel] Extending/adding to an R6 class from another package: qns


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://na01.safelinks.protection.outlook.com/?url=https%3A%2F%2Fadv-r.hadley.nz%2Foo-tradeoffs.html%23tradeoffs-pipe&data=02%7C01%7Chongooi%40microsoft.com%7C444ca3edfdc6470f026208d635d97c48%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636755605574903433&sdata=MQqYwnd7hWSEiEUeIFHUwzyo1SQ8R85Aq%2FpdqNW8ppg%3D&reserved=0)

> 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

-- 
https://na01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fhadley.nz&data=02%7C01%7Chongooi%40microsoft.com%7C444ca3edfdc6470f026208d635d97c48%7C72f988bf86f141af91ab2d7cd011db47%7C1%7C0%7C636755605574903433&sdata=sNTNDb%2BcxGHWvTkrhztq0%2F%2B6nqg97loNaU813Z5UN6s%3D&reserved=0