yonicd / sinew

Generate roxygen2 skeletons populated with information scraped from the function script.
https://yonicd.github.io/sinew/
Other
166 stars 15 forks source link

modify the default fields used in makeOxygen #9

Closed lbusett closed 7 years ago

lbusett commented 7 years ago

Hi,

first of all, thanks for a really useful package !

I'd like to know if there is a way to modify the default "add_fields" argument used by makeOxygen in a "sticky" way, so that I can call the function with some added fields (e.g. , author) without having to specify all the fields by hand each time.

If not already available, I'd propose that as a possible improvement.

This also because specifying an additional field of interest in add_fields currently overrides the defaults. Therefore, for example, to add the "author" field I have to enter:

add_fields = c("details", "examples",  "seealso", "rdname", "export", "author")

thanks in advance !

yonicd commented 7 years ago

Thank you for opening the issue. There are default fields in add_fields: c("details","examples","seealso","rdname","export"), ironically the oxygen header for makeOxygen was not updated (now it is) so they did not show up in the rd (help) file. I would also recommend using the moga and it's interactive addin in the development package (devtools::install_github(metrumresaerchgroup/sinew)). They update already made oxygen headers with new information that was changed in the body of the function.

lbusett commented 7 years ago

Hi, thanks for the answer. A couple of things:

  1. I probably explained myself poorly before .

I know that if I just call:

 makeOxygen(my_fun)

without other arguments, the "details", "examples", "seealso", "rdname" and "export" fields are automatically inserted. Suppose however that I wanted to also include "author": the naive approach would be just to use :

 makeOxygen(my_fun, add_fields = c("author"))

, but this would "remove" all the other "default" add_fields.

Therefore, if I want to include "author" but also keep the default add_fields I have to do:

makeOxygen(my_fun, add_fields = c("details", "examples",  "seealso", "rdname", "export", "author")

, which is long to write.

I think that it could be nice/useful if the user could save it's own default add_fields

An easy possibility could be to set an option within "R" which, if set, would override the default behaviour. Something like:

> options(sinew_defaultfields = c("details", "examples",  "seealso", "rdname", "export", "author"))
> options()$"sinew_defaultfields"
[1] "details"  "examples" "seealso"  "rdname"   "export"   "author"

, and then have a check at the beginning of makeOxygen to see if the option was set, and modify add_fields accordingly.

What do you think ? It would be a quite simple hack, and I think it would be nice to have. I could have a go at it and make a pull request to better show what I mean, if you want.

  1. I just tried out the interactive addin and it's really nice ! However, it seems that the "see_also" field is missing in the interface.

HTH,

Lorenzo

yonicd commented 7 years ago

I understand. I may not get to it that soon, but I like the idea. If you can PR that would be great.

I'll check the addin, but I think since it's added by default if makeoxygen uses importFrom is a reason it was omitted

yonicd commented 7 years ago

added. (there is a lingering bug that i cant get to work)

lbusett commented 7 years ago

Hi, it seems we were working on the same thing... I saw your message while sketching the PR. I did it nonetheless: Have a look and see if it is still useful. Consider that, as per PR message, I also added a (possible) new functionality, allowing to create a default also for the @author field, which could be auto-completed with a character string, possibly derived from a person object. Hope this still helps !

yonicd commented 7 years ago

i like the person addition. thank you for the PR and the ideas. I'll incorporate them to make the experience more user friendly

lbusett commented 7 years ago

Hi,

I checked on my fork (without merging your commit) and it seems to work properly (I see the preview immediately, without clicking any checkboxes).

yonicd commented 7 years ago

on ur version it should disappear when you add one checkbox

lbusett commented 7 years ago

i'll have a look.

lbusett commented 7 years ago

I reinstalled from github, and it seems to work well for me.

lbusett commented 7 years ago

By the way: I'm not sure that I get how you implemented the "set default" functionality: how would I change/set the default add_fields ?

yonicd commented 7 years ago

what OS are you using?

sinew_opts$set(list(add_fields=c('author')))

> sinew_opts$get()
$add_fields
[1] "author"

> sinew_opts$get('add_fields')
[1] "author"
yonicd commented 7 years ago

i still have some work on having the addin read the current fields from the header and checking them off when user goes into update mode.

lbusett commented 7 years ago

I'm on windows. I could check on Linux tomorrow at work.

On 16 Jul 2017 18:11, "yonicd" notifications@github.com wrote:

i still have some work on having the addin read the current fields from the header and checking them off when user goes into update mode.

— You are receiving this because you authored the thread.

Reply to this email directly, view it on GitHub https://github.com/metrumresearchgroup/sinew/issues/9#issuecomment-315619311, or mute the thread https://github.com/notifications/unsubscribe-auth/AERdjVU2Br6D2js9G2VYSek2yEY_jM5Zks5sOjYYgaJpZM4OY8oc .

yonicd commented 7 years ago

you are right. it does load right in windows. errr

yonicd commented 7 years ago

closing this issue.

lbusett commented 7 years ago

Hi, I had a look at the change, and now I understand the behaviour: nice ! (by the way: that "trick" for creating some sort of "methods" as a list of functions is nice - though maybe a little difficult to document - ).

If I can iterate a bit more on this, something that could be added is the possibility to save/load the options, so that they can be recycled among sessions without re-specifying them .

for example, adding this to defaults.R:

...  
...
save_opts = function(outfile) {
    myopts <- sinew_opts$get()
    save(myopts, file = outfile)
  }
  load_opts <- function(infile) {
    load(infile)
    sinew_opts$set(myopts)
  }

  list(get = get, set = set, merge = merge, restore = restore, 
       save_opts = save_opts, load_opts = load_opts)
}

would allow this kind of workflow:

> # Set to defaults
> sinew_opts$restore()
> sinew_opts$get()
$add_fields
[1] "details"  "examples" "seealso"  "rdname"   "export"  

> # Change the defaults
> sinew_opts$set(add_fields = c("author", "details"), author = "John Smith")
> sinew_opts$get()
$add_fields
[1] "author"  "details"

$author
[1] "John Smith"

> # Save the defaults
> sinew_opts$save_opts("/home/lb/Temp/myopts.RData")

> # Set to defaults again (or close and reopen R - which implicitly re-sets sinew_opts)
> sinew_opts$restore()
> sinew_opts$get()
$add_fields
[1] "details"  "examples" "seealso"  "rdname"   "export"  

> # Load previously saved options
> sinew_opts$load_opts("/home/lb/Temp/myopts.RData")
> sinew_opts$get()
$add_fields
[1] "author"  "details"

$author
[1] "John Smith"

What do you think ? I know re-setting the defaults is just a one-liner, but I think this could be useful (maybe just because I'm lazy...)

HTH,

Lorenzo

yonicd commented 7 years ago

how is this solution?

> sinew_opts$append(list(add_fields='author'))

> sinew_opts$get('add_fields')
[1] "details"  "examples" "seealso"  "rdname"   "export"   "author"  

> sinew_opts$set(list('author'=quote(person("Vladamir", "Putin", email = "dons@pptapes.com", role = c("aut", "cre")))))

> makeOxygen(moga)
#' @title FUNCTION_TITLE
#' @description FUNCTION_DESCRIPTION
#' @param path PARAM_DESCRIPTION
#' @param ... PARAM_DESCRIPTION
#' @param force.fields PARAM_DESCRIPTION, Default: NULL
#' @param dry.run PARAM_DESCRIPTION, Default: TRUE
#' @return OUTPUT_DESCRIPTION
#' @details DETAILS
#' @examples 
#' \dontrun{
#' if(interactive()){
#'  #EXAMPLE1
#'  }
#' }
#' @rdname moga
#' @export 
#' @author person("Vladamir", "Putin", email = "dons@pptapes.com", role = c("aut", "cre"))

this way using sinew_opts$set you can override default settings.

as for the persistency.

i shy away from that because it is kind of intrusive an action for a package to take. it would be something like creating/updating an ~/.Renviron file and then create options variables persistent across sessions.

lbusett commented 7 years ago

Looks nice ! Just one thing, since I didn't have the time to check it: is the "person" object parsed automatically when generating the "Rd", so that it is written as plain text in the documentation ?

If not, you could consider the trick I was using in my initial implementation:

if (inherits(author, "person")) {
  author <- as.character(paste(author, collapse = ";\\cr \n#' "))
}

to get a properly formatted "text" version of the person object in the parsed documentatoin.

Regarding your reluctance to set environment parameters, I think that you are right: it's different if I do it on my own or a package does it. However, I think that allowing the user to save his "options" to a file of his choice from which he can later one "load" them could be a good compromise, since it requires a specific action from the user (e.g., using explicitly sinew_opts$save). (That's obviously just my opinion, though. )

Keep up the good work !

yonicd commented 7 years ago

right now the user would need to just input the person class as quoted and it works fine w no need to create a helper fn internally.

sinew_opts$set(list('author'=quote(person("Vladamir", "Putin", email = "dons@pptapes.com", role = c("aut", "cre")))))

havent decided if that isnt intuitive enough. (even though w the intro of tidylang it should be straight forward.

I talked with a few more ppl about the opts$save/load, they also felt uncomfortable w it for reproducibility issues (much like why it's common practice now to turn off the default save .Rdata in Rstudio projects now)

lbusett commented 7 years ago

I think that it is quite intuitive, so no worries. My proposal was just meant to bea able to get something like this:

image

instead of this:

image

(that is, the "person" is parsed, and you get (for example) a working link for the email (which maybe is not a so good idea...) - also, missing the parenthesis on "roles".

However, I think that both are equally good.

Concerning the "save/load" question: No problem. In the end, I recon now that there' no need to implement an accessory function, since I can just do:

myopts <- sinew_opts$get()
save(myopts, file = "WIP/myopts.RData")

to save (if I want), and then:

sinew_opts$set(get(load("WIP/myopts.RData")))

to reload.