hadley / lazyeval

Lazy evaluation: an alternative to non-standard evaluation (NSE) for R
131 stars 40 forks source link

support for r 3.0.0 #87

Open javierluraschi opened 8 years ago

javierluraschi commented 8 years ago

Add support for r 3.0.0 to allow upstream packages like tibble to support 3.0.0 as well.

javierluraschi commented 8 years ago

@hadley some tests are failing with the port from the original shallod_duplicate function. Instead, seems we should look at porting a more recent version: https://github.com/wch/r-source/blob/af7f52f70101960861e5d995d3a4bec010bc89e6/src/main/duplicate.c

Now... it looks like shallow_duplicate and duplicate are now sharing code through duplicate1. Being that the case, we could replace the call to shallow_duplicate with duplicate. However, I don't fully understand what would be the side effects of using duplicate in older versions of R that do not provide a shallow copy.

Would this be a performance downgrade? Or would this break lazyeval.

Happy to keep this PR open but it definitely needs review from more experienced folks familiar in the r core codebase and lazyeval.

codecov-io commented 8 years ago

Current coverage is 97.27% (diff: 100%)

Merging #87 into master will decrease coverage by 1.02%

@@             master        #87   diff @@
==========================================
  Files            15         15          
  Lines           354        367    +13   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits            348        357     +9   
- Misses            6         10     +4   
  Partials          0          0          

Powered by Codecov. Last update c155c3d...c90aa66

hadley commented 8 years ago

It's probably a minor performance regression, but it seems like it's going to be challenging to make lazyeval backward compatible with R 3.0.0.