Roxygen2: Documentation for R6 methods added dynamically by "$set()"

Created on 5 Oct 2019  Â·  12Comments  Â·  Source: r-lib/roxygen2

I've just converted minihtml over to latest roxygen2 from github to test out the R6 support and it's working pretty well.

One challenge at the moment is that I generate many R6 methods for HTMLElement class dynamically using HTMLElement$set("public", method_name, method). I can not see any way to generate documentation for these methods and roxygen is very noisy in telling me that neither the methods nor any of their arguments are documented!


Click to see section of warnings generated during documentation step

(... snipped a 100 lines at the start ...)
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `title()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `tr()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `track()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `tt()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `ul()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `var()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `video()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] `wbr()`: undocumented R6 method
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] argument `...` undocumented for R6 method `a()`
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] argument `href` undocumented for R6 method `a()`
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] argument `hreflang` undocumented for R6 method `a()`
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] argument `media` undocumented for R6 method `a()`
Warning: [/Users/mike/projects/minihtml/R/HTMLElement.R:1] argument `rel` undocumented for R6 method `a()`

(... snipped a few hundred lines at the end ...)

Questions:

  • Is there a way to document methods added dynamically with a $set()?
  • Is there a way to tell roxygen2 to be quieter about missing documentation for methods?
  • Is there a way to tell roxygen2 that although some methods are public I do not want to include them in the documentation?

Sidenote: Whether or not it's a good idea to generate a method for each of the ~100 HTML tags is a different conversation :)

feature R6

Most helpful comment

Also, I think supporting set is an important feature to have for another reason: with complex R6 classes, it's often a maintainability plus to split the class over multiple source files. Without set, you'd have to implement the class as a single giant block of code. But this falls down if Roxygen doesn't support it.

All 12 comments

Is there a way to document methods added dynamically with a $set()?

Nope.

Is there a way to tell roxygen2 to be quieter about missing documentation for methods?

Nope, except for suppressWarnings() which you probably don't want.

Is there a way to tell roxygen2 that although some methods are public I do not want to include them in the documentation?

Nope.

All of these would be reasonable features, especially the first and the third. The third one seems easy, we could allow @keyword internal in the documentation of the method, and then these would be omitted. This does not really help you if the method is created with set().

The current scheme just assumed that all docs are in a single roxygen block, and the methods docs are inline.

@mikefc Please open issues for $set() and non-documented methods.

Edit: I guess this one is for $set(). :)

Looking at your use of $set() at https://github.com/coolbutuseless/minihtml/blob/46f1cf6c9c3243fff4fb50948161625a817fc5d8/R/htag.R#L264
I guess you don't want to add documentation for each method....

For this case I don't really want to add documentation for any of those methods. At the most I'd want to somehow lump them into a single documentation element which would just point the user to external documentation on the web.

Maybe a single roxygen warning would be a good compromise?

Edit: I mean, eventually we'll add support for set(), but for your use case, a single warning seems clearly better. And you can still add your special documentation manually for these classes.

On Sat, 5 Oct 2019, 12:11 mikefc, notifications@github.com wrote:

For this case I don't really want to add documentation for any of those
methods. At the most I'd want to somehow lump them into a single
documentation element which would just point the user to external
documentation on the web.

—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
https://github.com/r-lib/roxygen2/issues/931?email_source=notifications&email_token=AAFBGQA3MVZOWJSWAM2EE4TQNBY6TA5CNFSM4I5XFYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANQBFA#issuecomment-538640532,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAFBGQDUTQXCOX3JK3J6AJLQNBY6TANCNFSM4I5XFYSA
.

Re $set() support, I can already see some challenges.

One is that currently the R6 doc generator works at the block level, and it should really generate the output later at the topic level, so we can use data from multiple blocks. This is an implementation detail, and we can change it, it is mostly just refactoring.

The second is that I am not sure how we would detect $set() calls. They just return NULL AFAICT, so in

#' @description Foo.
#' @param bar Bar.
myclass$set('public', method_name, method_fun) 

roxygen needs to be able to tell that this call sets a method on an R6 class.

Another thing is that if a method is not added by the time the package env is loaded, but only later, then we'll not see it. I guess there is not much we can do about this.

I don’t think $set() returning null should be a problem - you can just inspect components of the call to find the right class.

I don’t think $set() returning null should be a problem - you can just inspect components of the call to find the right class.

For the simple cases, yes. But $set() does allow some very funky stuff as well. E.g.

.onLoad <- function(libname, pkgname) {
  #' @param x, y params
  myfun <- function(x, y) { }
  myclass$set('public', 'mymethod', myfun)
}

which is probably not something we want to support. I guess the challenge is to investigate the forms we do want to support.

Anyway, I don't think $set() support is that urgent, the OP does not actually want to document all the 100+ dynamically added methods.

FYI, this will not make it into the next roxygen release, but we still want to implement it later.

Glad you’re planning on implementing this! Will be super helpful for the new stan-dev/cmdstanr which we’ll release with r6=false or converting every method to being defined with the class so we can use inline doc, but looking forwards to this.

And thanks for the R6 functionality you’ve already inciuded!

Something else to consider as well: it's possible for a package to use set to add new members to a class defined in a different package. I use this a lot in my AzureR packages, to extend a small set of base classes to support various Azure services.

This was previously mentioned in #807 but prematurely closed.

Also, I think supporting set is an important feature to have for another reason: with complex R6 classes, it's often a maintainability plus to split the class over multiple source files. Without set, you'd have to implement the class as a single giant block of code. But this falls down if Roxygen doesn't support it.

Was this page helpful?
0 / 5 - 0 ratings