Hooks

[ropensci]

Hooks

[wlandau]

We could also use tidyselect to choose which targets to apply the hooks.

[wlandau]

Also, let's consider supporting arbitrary expressions instead of just single-argument functions. A hook could look something like tar_hook_outer(target_object, hook = run_in_docker(code = .x, hostname = "localhost")), and the wrapped command could go in .x.

[wlandau]

EDIT

Please see #44 (comment). I decided to veer away from in-place modifications.

original post

A few implementation issues that will probably come up repeatedly in subsequent discussions:

Composition of target lists

Should we return a flattened list of target objects, e.g. unlist(list(targets), recursive = TRUE), or should we keep the target list nested in the way the user defines it originally, e.g. tar_map(unlist = FALSE)? I would think the latter. This outcome seems more likely to prevent surprises. Hooks should be minimally invasive when possible.

Modification in place

Should the hooks modify the target objects in place, or should they return deep copies of those objects? If we were to make deep copies, we would probably need to flatten the target list. (Recursing over an arbitrarily nested list and making arbitrary changes requires passing by reference unless we come up with some messy index scheme to keep track of where we are in the list at all times.) Above, we already decided against flattening the target list, so we should go with in-place modifications.

Return value

Given the decision to go with modification in place, we have a choice when it comes to the return value. We could either return the modified target list, or we could invisibly return NULL. I strongly prefer the latter because it emphasizes that target objects are modified in place. The behavior is limiting, and it prevents the use of the magrittr pipe, but it is a strong guardrail that I believe will prevent egregious and counterintuitive user errors.

Drawback: potential to accidentally drop targets

Users might not read the docs and incorrectly assume tar_hook_before() returns target objects. In that case, those targets may not make it into the pipeline. This kind of mistake is not so costly because no extra runtime is spent on misspecified targets, so I think this is the right tradeoff.

Drawback: metaprogramming limitations

Users might want to apply multiple hooks to a common template target object. However, this is already possible with tar_map(). The template can be created with tar_target() and possibly tar_hook_first() etc., and then tar_map() could be invoked afterwards.

Best alternative

The best alternative I see to all this is to return a flattened list of deep-copied target objects. I still prefer not to do this because

1. It would be more invasive, and
2. Later on, if the demand is high enough, we can always build wrappers on top of tar_hook_first() etc. that make deep copies beforehand. We could either name those functions tar_hook_first_copy() etc. or think of another term besides "hook". But let's try to pursue slick workarounds before we jump to an implementation.

[wlandau]

TL;DR: I am trying to make hooks safe, even if that makes them a bit limited (at first).

[wlandau]

On second thought, I think it's fine if hooks flatten the target list. I don't think people need the actual structure in a target list all that often.

[wlandau]

So #44 (comment) is a justification for flattening the target list, not modification in place.

[wlandau]

Implemented with the deep copy route.