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

detect() doesn't seem to take a logical vector #348

Closed
rosseji opened this issue Jul 14, 2017 · 6 comments
Closed

detect() doesn't seem to take a logical vector #348

rosseji opened this issue Jul 14, 2017 · 6 comments

Comments

@rosseji
Copy link

rosseji commented Jul 14, 2017

Documentation reads:

.p
A single predicate function, a formula describing such a predicate function,
or a logical vector of the same length as .x

But...

x <- list("foo", "bar")

stringr::str_detect(x,"f")
#> [1]  TRUE FALSE

purrr::detect(x, stringr::str_detect(x,"f"))
#> Error: Don't know how to convert logical into a function

Saw, #330 and see that there might be better functions, but I'm still curious...
A simple work around:

x <- list("foo", "bar")

is_foo <- function(x) stringr::str_detect(x, "f")

purrr::detect(x, is_foo)
#> [1] "foo"

Thanks

sessionInfo()
#> R version 3.4.0 (2017-04-21)
#> Platform: x86_64-apple-darwin15.6.0 (64-bit)
#> Running under: macOS Sierra 10.12.5
#> 
#> Matrix products: default
#> BLAS: /Library/Frameworks/R.framework/Versions/3.4/Resources/lib/libRblas.0.dylib
#> LAPACK: /Library/Frameworks/R.framework/Versions/3.4/Resources/lib/libRlapack.dylib
#> 
#> locale:
#> [1] en_AU.UTF-8/en_AU.UTF-8/en_AU.UTF-8/C/en_AU.UTF-8/en_AU.UTF-8
#> 
#> attached base packages:
#> [1] stats     graphics  grDevices utils     datasets  methods   base     
#> 
#> other attached packages:
#> [1] dplyr_0.7.1     purrr_0.2.2.2   readr_1.1.1     tidyr_0.6.3    
#> [5] tibble_1.3.3    ggplot2_2.2.1   tidyverse_1.1.1
#> 
#> loaded via a namespace (and not attached):
#>  [1] Rcpp_0.12.11     cellranger_1.1.0 compiler_3.4.0   plyr_1.8.4      
#>  [5] bindr_0.1        forcats_0.2.0    tools_3.4.0      digest_0.6.12   
#>  [9] lubridate_1.6.0  jsonlite_1.5     evaluate_0.10.1  nlme_3.1-131    
#> [13] gtable_0.2.0     lattice_0.20-35  pkgconfig_2.0.1  rlang_0.1.1     
#> [17] psych_1.7.5      yaml_2.1.14      parallel_3.4.0   haven_1.1.0     
#> [21] bindrcpp_0.2     xml2_1.1.1       httr_1.2.1       stringr_1.2.0   
#> [25] knitr_1.16       hms_0.3          rprojroot_1.2    grid_3.4.0      
#> [29] glue_1.1.1       R6_2.2.2         readxl_1.0.0     foreign_0.8-69  
#> [33] rmarkdown_1.6    modelr_0.1.0     reshape2_1.4.2   magrittr_1.5    
#> [37] backports_1.1.0  scales_0.4.1     htmltools_0.3.6  rvest_0.3.2     
#> [41] assertthat_0.2.0 mnormt_1.5-5     colorspace_1.3-2 stringi_1.1.5   
#> [45] lazyeval_0.2.0   munsell_0.4.3    broom_0.4.2
@david-jankoski
Copy link

i understood the docs as .p should always be a function. Maybe you trying to show a different point but the way you are trying to put purrr::detect() to use could be simply x[stringr::str_detect(x,"f")] right ?

@rosseji
Copy link
Author

rosseji commented Jul 19, 2017

No, you could be right... I guess that would mean that the documentation should be updated if .p should be a function.

Thanks for the suggestion... Can it be map()ed?

@david-jankoski
Copy link

It seems to work so. As far as i understand the error comes when detect passes your logical vector (the result of your call to str_detect) to rlang::as_function. If the guys think a clarification in the docs is worthy i can try my hand at it.
About your question - would be better if you post it on stack overflow, expand on what exactly you are trying to do and i can try and help you.

@lionel-
Copy link
Member

lionel- commented Jul 19, 2017

It seems like .p should be called .f. I think the original intent is to have mapper semantics, so that this works:

x <- list(
  list(1, foo = FALSE),
  list(2, foo = TRUE),
  list(3, foo = TRUE)
)

detect(x, "foo")
detect_index(x, "foo")

lionel- added a commit to lionel-/lowliner that referenced this issue Jul 19, 2017
So they are documented with the right semantics. Closes tidyverse#348
lionel- added a commit to lionel-/lowliner that referenced this issue Jul 19, 2017
So they are documented with the right semantics. Closes tidyverse#348
@rosseji
Copy link
Author

rosseji commented Jul 20, 2017

@david-jankoski Thanks, I'll leave it here. Appreciate the comments thought

And thanks @lionel-, got it.

@jpcompartir
Copy link

After changing detect from .p to .f, the documentation still refers to .p for ellipsis

.f | A function, specified in one of the following ways:
... | Additional arguments passed on to .p.

small thing but perhaps worth changing?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants