Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Informative error message when "primary pipe input" is missing #4127

Closed
jankowtf opened this issue Jan 25, 2019 · 6 comments
Closed

Informative error message when "primary pipe input" is missing #4127

jankowtf opened this issue Jan 25, 2019 · 6 comments

Comments

@jankowtf
Copy link

jankowtf commented Jan 25, 2019

Just a really minor thing I noticed (and possibly this is rather a magrittr issue?):

While refactoring a function, I accidentally dropped the initial df %>% part of a pipe that used :=.

But before I realized this, I was puzzled for a couple minutes by the error below as it didn't suggest the reason was the missing df %>% but not being able to find function :=.

Maybe one could come up with error messages along the lines of primary pipe input 'x' missing?

library(magrittr)

df <- tibble::tibble(x = 1:3)
col <- rlang::sym("x")

df %>% 
  dplyr::mutate(!!col := !!col * 10)
#> # A tibble: 3 x 1
#>       x
#>   <dbl>
#> 1    10
#> 2    20
#> 3    30

# When accidentally dropping `df %>%`:
  dplyr::mutate(!!col := !!col * 10)
#> Error in `:=`(!!col, !!col * 10): could not find function ":="
@romainfrancois
Copy link
Member

Unfortunately, I don't think there is much we can do here, as the error happens before S3 dispatch, i.e. the first argument has to be evaluated before the mutate generic knows which method to call.

@krlmlr
Copy link
Member

krlmlr commented Jan 29, 2019

We can do a tryCatch() in the mutate() generic:

mutate <- function(.data, ...) {
  quo <- enquo(.data)

  tryCatch(
    force(.data),
    error = function(e) {
      abort(paste0("`.data` must be a data source. ", conditionMessage(e)))
    }
  )

  UseMethod("mutate")
}

This doesn't look very helpful right now, perhaps if we had a tryCatch() implementation with better call traces:

dplyr::mutate(!!col := !!col * 10)
#> Error: `.data` must be a data source. could not find function ":="
#> Backtrace:
#>     █
#>  1. └─dplyr::mutate(`:=`(!!col, !!col * 10))
#>  2.   └─base::tryCatch(...) /home/kirill/git/R/dplyr/R/manip.r:442:2
#>  3.     └─base:::tryCatchList(expr, classes, parentenv, handlers)
#>  4.       └─base:::tryCatchOne(expr, names, parentenv, handlers[[1L]])
#>  5.         └─value[[3L]](cond)

Created on 2019-01-29 by the reprex package (v0.2.1.9000)

@romainfrancois
Copy link
Member

We'd have to do that for every generic then. Sounds painful.

@hadley
Copy link
Member

hadley commented Jan 29, 2019

The cost-benefit tradeoff does not favour adding a tryCatch() here.

@krlmlr
Copy link
Member

krlmlr commented Jun 29, 2019

We do this in ggplot(), and it really helps, especially for beginners. The tryCatch() could be extracted into a helper function, so we only need to remember to call that function (using the new whiskers syntax in rlang) at the beginning of each generic.

@lock
Copy link

lock bot commented Dec 26, 2019

This old issue has been automatically locked. If you believe you have found a related problem, please file a new issue (with reprex) and link to this issue. https://reprex.tidyverse.org/

@lock lock bot locked and limited conversation to collaborators Dec 26, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants