MilesMcBain / friendlyeval

A friendly interface to tidyeval/rlang that will excuse itself when you're done.
Other
107 stars 6 forks source link

Copying functions from rlang now fixes version on installation #2

Open JoFAM opened 6 years ago

JoFAM commented 6 years ago

If you do the following:

typed_as_name <- rlang::enquo

the function code is copied from rlang into your own package at time of installation. That also means that when an updated version of rlang is installed by the user, your function typed_as_name still uses the old rlang::enquo code. The prefered (and documented) way to get around this issue, is to wrap it in a function (see eg explanation in Writing R extensions:

typed_as_name <- function(arg) rlang::enquo(arg)

But in that case typed_as_name won't work any longer due to dplyr::mytate_impl() not being able to process arg correctly. Same happens when you try with

typed_as_name <- function(arg) eval.parent(rlang::enquo(arg))

Given the nature of the function rlang::enquo, this issue is pretty hard to get around. Then again, these rlang functions are unlikely to change so this shouldn't really be a problem in everyday use.

MilesMcBain commented 6 years ago

Thanks for letting me know about this! What about setting the functions using the on load hook as an idea?

MilesMcBain commented 6 years ago

Although everything will still work if user uses the conversion feature. So maybe it's simplest to do nothing.

JoFAM commented 6 years ago

Thought about it, but when using .onLoad one should not assume any package except the base package to be on the search path. So pretty sure that this is opening a can of worms you rather keep closed.

I couldn't come up with something either, so "do nothing but keep in mind this might be an issue at one point" would be my choice too.

lionel- commented 6 years ago

When you use .onLoad() you can assume any package in your Imports: is loaded (the search path doesn't matter here). This is the right time to populate aliases in your namespace at load time rather than build time. Joris is right that the latter is problematic.

MilesMcBain commented 6 years ago

Thanks for the clarification Lionel!