-
Notifications
You must be signed in to change notification settings - Fork 235
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Documentation for R6 methods added dynamically by "$set()" #931
Comments
Nope.
Nope, except for
Nope. All of these would be reasonable features, especially the first and the third. The third one seems easy, we could allow 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 Edit: I guess this one is for |
Looking at your use of |
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, ***@***.***> 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
<#931?email_source=notifications&email_token=AAFBGQA3MVZOWJSWAM2EE4TQNBY6TA5CNFSM4I5XFYSKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEANQBFA#issuecomment-538640532>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAFBGQDUTQXCOX3JK3J6AJLQNBY6TANCNFSM4I5XFYSA>
.
|
Re 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 #' @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 |
For the simple cases, yes. But .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 |
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 This was previously mentioned in #807 but prematurely closed. |
Also, I think supporting |
Any current plans to add support for this? |
For our use case, it would also be helpful to have the class' methods documented in separate files--there are a lot of them, hence our pattern of splitting them into multiple R files using |
At the very least, it would be nice not to be spammed with warnings for not properly documenting methods that roxygen provides no way to properly document. |
You can set |
How should we format the documentation then? |
Here is an example that we use before roxygen2 had any R6 support: https://github.com/r-lib/processx/blob/v3.4.2/R/process.R#L5 |
Ah I see, just document |
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 usingHTMLElement$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
Questions:
$set()
?Sidenote: Whether or not it's a good idea to generate a method for each of the ~100 HTML tags is a different conversation :)
The text was updated successfully, but these errors were encountered: