nick-ulle/rstatic

Selecting indices from subset assignment

clarkfitzg opened this issue · 7 comments

I'd like to pick the indices out of a subset assignment. That is, for the line:

x[i, j, k] = foo

I want an argument list of [i, j, k]. Right now the argument list isn't named. I can get the indices by dropping the first and last arguments. This is acceptable, but I think it could be improved.

I would like it best if the argument list had semantically meaningful names, for example something like:
[original = x, indices = [i, j, k], value = foo].

Looking at the documentation for [<-, we see:

     x[i] <- value
     x[i, j, ...] <- value

This suggests another option- to do something like the match.call for the subset assignment, so that the arguments have names x, i, j, dots, value. This does raise some issues, because x[i] is different from x[i,] (with j missing).

Current behavior:

> e3 = quote_ast(x[1, 1] <- foo)
> e3
<Replacement> $parent $read $write
x = `[<-`(x, 1, 1, foo)

> e3$read
<Call> $args $fn $parent
`[<-`(x, 1, 1, foo)

> e3$read$args
<ArgumentList> $contents $parent
list(x, 1, 1, foo)

Rather than naming the arguments, the way I want to do this is by adding a method to get the indexes from calls to [, [[, [<-, etc.

I've gone ahead and done so (see get_index) in the commits leading up to c63321b. Empty arguments produce an EmptyArgument object, and these are returned by get_index. If you want names on the arguments, use match_call before using get_index.

I've also added subclasses for the different subset operators, which is something Duncan asked about a while ago.

Let me know whether this solves the issue. Adding similar methods to get just the object being subsetted or just the value (in the case of Replacement) seems reasonable to me, but I'm not sure what we'd call them since get_object and get_value are ambiguous.

get_index does just what I wanted, thanks.

It might need a different name though- there's another get_index in find_nodes.R

Does it make sense to add a get_index method for ReplacementDollar? How would we extract the foo out of here?

x = quote_ast(x$foo <- 100)

# Currrent way:
x$read$args$contents[[2L]]
<Symbol> $basename $name $namespace $namespace_fn $parent $ssa_name $ssa_number $value
foo

I did add a method for ReplacementDollar, but there was a typo in the method name. Fixed in 746039c.

Thanks for the heads up about the other get_index. I've moved it to where_is in ad3bb2b.

Looks good, thank you!

In 4cef3da, I moved get_index to arg_index to make the name less ambiguous. The get_index function is still there but deprecated, to be removed in a later version.

I also added other arg_ methods to get the object and value in subset/replacement as mentioned above. I'm planning to add support to these for slots (@) in the future as well.

You're right, arg_index is more precise than get_index. I changed my code, go ahead and remove it.