cfhammill / lenses

Elegant Data Manipulation with Lenses
https://cfhammill.github.io/lenses
Other
27 stars 4 forks source link

hilarious tidyeval sadness #31

Open bcdarwin opened 5 years ago

bcdarwin commented 5 years ago

aka why R can't have nice things ...

iris %>% view(filter_l(.[names(.)[1]] > 7))
cfhammill commented 5 years ago

Although you are correct that there is a problem, this is not it. The constructed filter here refers explicitly to the values in iris prior to any modification. This use is in fact lawful :stuck_out_tongue:.

your example becomes:

view(iris, filter_l(iris[names(iris)[1]] > 7))

the lens in there will always hit the same rows, so set-view is intact.

There is a way to trigger this bug, but you need to do meta-programming inside your filter expression.

I also have a proof of concept for the fix:

https://gist.github.com/cfhammill/4244e29bc667455a2406bf972e4287c6

Which is pretty awesome.

bcdarwin commented 5 years ago

I wasn't claiming that a lens law was violated here, although I'm not surprised that it's possible. Even more serious: this is actually a violation of equational reasoning in which the actual columns returned depends on whether you obfuscate the names of the ones you're interested in :)

cfhammill commented 5 years ago

I don't understand. Here you get Sepal.Length in the output because the Sepal.Length in your input data isn't being referenced. Here you're constructing a filter equal to

  [1] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [13] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [25] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [37] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [49] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [61] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [73] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [85] FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
 [97] FALSE FALSE FALSE FALSE FALSE FALSE  TRUE FALSE FALSE  TRUE FALSE  TRUE
[109] FALSE  TRUE FALSE FALSE FALSE FALSE FALSE FALSE FALSE  TRUE  TRUE FALSE
[121] FALSE FALSE  TRUE FALSE FALSE  TRUE FALSE FALSE FALSE  TRUE  TRUE  TRUE
[133] FALSE FALSE FALSE  TRUE FALSE FALSE FALSE FALSE FALSE FALSE FALSE FALSE
[145] FALSE FALSE FALSE FALSE FALSE FALSE

which will fail if you apply it to a data set with fewer than 150 rows. The . in the filter condition is not the data in view. Well it is in this case but only by coincidence.

bcdarwin commented 5 years ago

Ah, you're right about this example.

cfhammill commented 5 years ago

Definitely still breakable though, but now it seems like you at least have to try