Implement rmodify()

[hadley]

Fixes #720

@vspinu here's a concrete proposal for rmodify() that I'd love to get your feedback on.

To do:

• News bullet
• Tests
• Update examples once Use R 4.1 examples in docs #938 is merged

[vspinu]

It's not immediately clear what transformation is meant by "transformed, nor the relationship to the tree traversal.

I would clarify this statement further along the lines ".. applied to each node when the tree is traversed down, aka before the leaves are transformed with f_leaf.

[vspinu]

This actually made me thinking, is it necessary to apply f_pre and f_post to leafs? I think most commonly the f_pre and f_post will be used on nodes and in general the same operation is unlikely to be applied on both leafs and nodes. As a result a user would need to add an extra check for leafs inside f_pre and f_post.

Also applying all 3 transformations on leafs is redundant and slightly confusing.

[hadley]

I’m glad you bought that up because that’s what I was thinking too 😀

[hadley]

Hmmmm, what happens if f_pre changes x from a node to a leaf?

[hyiltiz]

As a map-like function, I think it should preserve the types/shape of the data structure; f_pre/f_post should have Node -> Node type and should not delete a node or transmute a node into a leaf. Otherwise, the behavior is a bit surprising, depending on how the tree is traversed.

[vspinu]

I think being able to change a type of a node is a very important feature. For example one might want to change the end list nodes (prior to leafs) into a proper atomic vector.

I find it hard to think of good use-cases for f_pre, but the one you mentioned seem an important one. It's quite possible that the purpose of an operation is to transform a sub-tree into a leaf. The traversal stops right there. This can be important for deeply nested trees where performance starts playing a role. On the other hand p_leaf can capture this use case as well.

So there are 4 options:

1. leave as it is and add a note that f_pre should return a node, otherwise the behavior is undefined.
2. add an extra check for leaf after f_pre
3. make f_pre/f_post asymetric and apply f_pre to the leaf node but not f_post
4. remove f_leaf and add extra parameter to f_post/f_pre, leaf=TRUE

I am somewhat indifferent between 1 and 2. Though 2 seems a bit cleaner conceptually and the default penalty of is.list is trivial.

[lionel-]

I think it'd be more consistent if we checked for a leaf and return leaf(x) in that case.

[DavisVaughan]

Why don't we require x to be a list with vec_check_list()?

I feel like allowing non-lists isn't useful because pre/post won't ever be applied? I guess that depends on what p_leaf() returns though (i.e. in theory it could say a numeric value is a node, but i feel like that isn't useful)

[hadley]

I think there's probably some invariant around modify_tree(x, f_pre = \(y) modify_tree(y, f_leaf)) == modify_tree(x, f_leaf)

[vspinu]

My preference would be to allow for as wide class of objects as possible. @lionel-'s point about allowing rmodify on all objects that [[<- is allowed on makes sense to me.

Use cases:

• Side effectful recursive modification of nested environments
• Recursive processing of language objects. For instance, if one wants to modify specific parts of a function.

[DavisVaughan]

Why don't we require x to be a list with vec_check_list()?

I feel like allowing non-lists isn't useful because pre/post won't ever be applied? I guess that depends on what p_leaf() returns though (i.e. in theory it could say a numeric value is a node, but i feel like that isn't useful)

[DavisVaughan]

Neat, it's like list_squash()