moodymudskipper / fastpipe Goto Github PK

Though we pass all magrittr's tests, it doesn't mean we always give the same results.

A section about breaking changes in the readme would be nice, it would repeat some of the other info from readme but I think it's ok.

• Show how the behavior with !!!. changes the way we should call dplyr::count() #6
• Mention that functional sequences are not subsetable anymore, and that they don't have a class
• Mention that we don't have aliases (and other exported functions ?), but that the user can attach both magrittr and fastpipe if they want them
• ... ?

As I did with dotdot:`:=`

It needs to work on any package that attaches the pipe though so might be a bit different.

Need to add a mention about it in the README too

in readme and description

In the README,

remotes::install_github("fastpipe")

should be

remotes::install_github("moodymudskipper/fastpipe")

I sometimes use foo %>% dplyr::count(!!!.) for foo %>% dplyr::count(., !!!.)

It will be broken by fastpipe.

current doc says :

it's is unlikely that a user will nedd to use both . and !!!. in a call.

This exception might be integrated in the readme.

library(magrittr)
test <- tibble::tibble(a = letters[1:3], b = c(1,1,2))
test %>% dplyr::count(!!!.)
#> # A tibble: 3 x 3
#>   a         b     n
#>   <chr> <dbl> <int>
#> 1 a         1     1
#> 2 b         1     1
#> 3 c         2     1
library(fastpipe)
#> 
#> Attaching package: 'fastpipe'
#> The following objects are masked from 'package:magrittr':
#> 
#>     %$%, %<>%, %>%, %T>%
# not working anymore
test %>% dplyr::count(!!!.)
#> Error in FUN(left): invalid argument type
# easy fix
test %>% dplyr::count(., !!!.)
#> # A tibble: 3 x 3
#>   a         b     n
#>   <chr> <dbl> <int>
#> 1 a         1     1
#> 2 b         1     1
#> 3 c         2     1

^{Created on 2019-10-03 by the reprex package (v0.3.0)}

I think the feature is worth it, but README

Maybe I can find those packages in a systematic way...

I guess they use usethis::use_pipe() so maybe the created code would be searchable ?

Anyway we can already add stringr, forcats, rvest, modelr, testthat to the list

I think it can work a bit like the other special features.

When we hit the %B>% pipe in the call stack we switch the global variable browsing, when browsing is switched on we get to browse after each call, but where ?

We could wrap the *RETURN_CALL* in a function and debug there when browsing is switched on. When continuing we'd just get to the next browser call and so on (maybe no way to get out of all browser calls at once)

See if impact on speed is dramatic.

moodymudskipper/pipe#1

Having 2nd thoughts that the current pipe is the right general approach, maybe this one is better as it's more robust and we don't really need new pipes that badly.would be nice to have some kind of debugging pipe still though.

We'd remove the fastpipe.bare option and have a family of %>% pipes and a family of %>>% pipes, %>% is always consistent with magrittr, more robust and faster, and %>>% is fast and simple.

a functional sequence . %>% foo() %>% bar() will create a function function() . %>>% foo() %>>% bar() which is more efficient and looks much cleaner.

We make sure to keep the current pipe and to add at least the logging pipe, maybe not more, too keep it simple.

We can reasonably easily implement a lazy pipe, there's just a weird issue with functional sequences :

library(fastpipe)

`%~>%` <- function (lhs, rhs) 
{
  if (globals$master) {
    sc <- sys.call()
    return(eval_slaves(sc, parent.frame()))
  }
  lhs_call <- substitute(lhs)
  rhs_call <- insert_dot(substitute(rhs))

  if (lhs_call == quote(.)) {
    bare_pipe <- attr(sys.function(), "bare_version")
    res <- call(bare_pipe, lhs_call, rhs_call)
    fs_on()
    return(res)
  }

  if (globals$is_fs) {
    bare_pipe <- attr(sys.function(), "bare_version")
    res <- call(bare_pipe, lhs, rhs_call)
    return(res)
  }

  new_env <- new.env()
  eval(bquote(
    delayedAssign(".", .(lhs_call),eval.env = parent.frame(), assign.env = new_env)))
  eval(rhs_call, envir = new_env, enclos = parent.frame())
}

`%~>>%` <- function (lhs, rhs) {
  lhs_call <- substitute(lhs)
  rhs_call <- substitute(rhs)
  new_env <- new.env()
  eval(bquote(
    delayedAssign(".", .(lhs_call),eval.env = parent.frame(), assign.env = new_env)))
  eval(rhs_call, envir = new_env, enclos = parent.frame())
}

class(`%~>%`) <- class(`%~>>%`) <- "fastpipe"
attr(`%~>%`, "bare_version") <- "%~>>%"

environment(`%~>%`) <- as.environment(asNamespace("fastpipe"))
environment(`%~>>%`) <- as.environment(asNamespace("fastpipe"))


stop("!") %>%
  tryCatch(error = function(e) "An error")
#> Error in force(lhs): !

stop("!") %~>%
  tryCatch(error = function(e) "An error")
#> [1] "An error"

stop("!") %~>>%
  tryCatch(error = function(e) "An error")
#> [1] "An error"

"!" %>% stop() %~>%
  tryCatch(error = function(e) "An error") %>%
  toupper()
#> [1] "AN ERROR"

fs <- . %>% stop() %~>% 
  tryCatch(error = function(e) "An error") %>%
  toupper()

# this is weird, maybe just a namespace thing, try in package
fs
#> function (.) 
#> . %>>% stop(.) %>>% toupper(.)

# what if it had worked ?
fs <- function(.)
  . %>>% stop(.) %~>>% 
  tryCatch(error = function(e) "An error") %>>%
  toupper(.)

# then as expected
fs("!")
#> [1] "AN ERROR"

msg <- "old"
delayedAssign("x", stop("hi!"))
substitute(x) # shows only 'x', as it is in the global env.
#> x
msg <- "new!"
x # new!
#> Error in eval(expr, envir, enclos): hi!

^{Created on 2019-11-08 by the reprex package (v0.3.0)}

I think :

• Having all these pipes is overkill
• It is confusing
• %>>>% is too long to pipe

It should be enough to have drop in replacement and bare pipes, the former for interactive use and the latter for programming.