Re: [R-pkg-devel] Extending/adding to an R6 class from another package: qns
--- Begin Message --- 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 Sent: Saturday, 20 October, 2018 2:42 AM To: Hong Ooi Cc: R Package Development 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 --- End Message --- __ R-package-devel@r-project.org mailing list https://stat.ethz.ch/mailman/listinfo/r-package-devel
Re: [R-pkg-devel] Extending/adding to an R6 class from another package: qns
> 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
__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel
[R-pkg-devel] Extending/adding to an R6 class from another package: qns
--- Begin Message ---
I'm writing a family of packages for talking to Azure (Microsoft's cloud
service) from R. The basic architecture is
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?
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?
Hong
--- End Message ---
__
R-package-devel@r-project.org mailing list
https://stat.ethz.ch/mailman/listinfo/r-package-devel
