Skip to content
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

Improving .Rd doc for DSL and target() #979

Closed
2 tasks done
lorenzwalthert opened this issue Aug 6, 2019 · 5 comments
Closed
2 tasks done

Improving .Rd doc for DSL and target() #979

lorenzwalthert opened this issue Aug 6, 2019 · 5 comments

Comments

@lorenzwalthert
Copy link
Contributor

Prework

Description

I think the DSL in drake is interesting, but it is hard to learn how to use it. Lets take this example from the drake manual:

 drake_plan(
  data = get_data(),
  analysis = target(
    fun(data, mean = mean_val, sd = sd_val),
    transform = cross(mean_val = c(2, 5, 10, 100, 1000), sd_val = !!lots_of_sds)
  )
)

Neither does ?target, tell me about the argument transform nor can I use ?cross and similar for other DSL verbs to get to the documentation for these functions. They are very much magic to me. I consider the R documentation system a core strength of the eco system and hence I'd propose to extend the documentation in that regard. If ... is used in target(), then at least the doc could say where to look for the argument names if possible. Ideally adding examples to the Rd doc, but even linking to the drake manual could be an improvement of the status quo I think. Thanks for making this package so great.

@wlandau
Copy link
Collaborator

wlandau commented Aug 6, 2019

Yeah, I agree to extend the docs. map(), cross(), etc. are not actually functions, and target() is no longer supposed to be a user-side function, but we should still add *.Rd files.

@wlandau wlandau closed this as completed in 7099f27 Aug 9, 2019
@lorenzwalthert
Copy link
Contributor Author

Great, thanks a lot @wlandau.

@wlandau
Copy link
Collaborator

wlandau commented Oct 17, 2019

Reconsidering how we implement this. Problems:

  1. Name conflicts with true functions, e.g. purrr::map().
  2. Function arguments will be different in dynamic branching.

@wlandau wlandau reopened this Oct 17, 2019
wlandau-lilly added a commit that referenced this issue Oct 17, 2019
@lorenzwalthert
Copy link
Contributor Author

what about adding a prefix to the functions and soft-depreciate the initial API? Like

  • map -> dmap (or drake_map())
  • cross -> dcros
  • etc.

I think for many this is a source of confusion.

@wlandau
Copy link
Collaborator

wlandau commented Oct 17, 2019

The current drake::map() etc. just throw an informative error, so I do not think there is anything meaningful to deprecate here. The help links like ?map will still work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants