Skip to content

Commit dca5b5a

Browse files
ErdaradunGazteaDavisVaughanhadley
authored
Rewrite every(), some(), and none() in C (#1169)
* Initial implementation of quick every() * Extract C checks to a separate .C file * Implement some() in C * Implement none() in C Turns out that negate() adds too much overhead with C implementation of every() * Revert .gitignore modifications * Rework `every()`, `some()`, `none()` C implementations * Check revdeps * Rename to `elt_sexp` * Add comment and rely on bool to int conversion for clarity --------- Co-authored-by: Davis Vaughan <davis@posit.co> Co-authored-by: Hadley Wickham <h.wickham@gmail.com>
1 parent 0b958df commit dca5b5a

File tree

14 files changed

+1146
-716
lines changed

14 files changed

+1146
-716
lines changed

‎NEWS.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,14 @@
88

99
* `map_chr()` no longer coereces from logical, integer, or double to strings.
1010

11+
* `every()`, `some()`, and `none()` now require that `.p` return logical scalar `TRUE`, `FALSE`, or `NA`. Previously, `NA` was allowed to be a non-logical `NA`, and would be coerced to a logical `NA`.
12+
1113
## Minor improvements and bug fixes
1214

1315
* New "getting started" vignette, `vignette("purrr")` (#915, @ogolovkina).
1416

17+
* `every()`, `some()`, and `none()` are now more performant. They are now as fast as or faster than their equivalent `any(map_lgl())` or `all(map_lgl())` calls (#1036, @ErdaradunGaztea).
18+
1519
* `as_mapper.default()` optimized by removing special named argument handling for primitive functions (@mtcarsalot, #1088).
1620

1721
* `list_flatten()` gains an `is_node` parameter taking a predicate function that determines whether an input element is a node or a leaf (@salim-b, #1179).

‎R/every-some-none.R‎

Lines changed: 34 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -22,39 +22,48 @@
2222
#' # unsafe (e.g. in `if ()` conditions), make sure to use safe predicates:
2323
#' if (some(list(NA, FALSE), rlang::is_true)) "foo" else "bar"
2424
every <- function(.x, .p, ...) {
25-
.p <- as_predicate(.p, ..., .mapper = TRUE, .allow_na = TRUE)
26-
27-
val <- TRUE
28-
for (i in seq_along(.x)) {
29-
val <- val && .p(.x[[i]], ...)
30-
31-
if (is_false(val)) {
32-
return(FALSE)
33-
}
34-
}
35-
36-
val
25+
satisfies_predicate(.x, .p, ..., .purrr_predicate = "every")
3726
}
3827

3928
#' @export
4029
#' @rdname every
4130
some <- function(.x, .p, ...) {
42-
.p <- as_predicate(.p, ..., .mapper = TRUE, .allow_na = TRUE)
43-
44-
val <- FALSE
45-
for (i in seq_along(.x)) {
46-
val <- val || .p(.x[[i]], ...)
47-
48-
if (is_true(val)) {
49-
return(TRUE)
50-
}
51-
}
52-
53-
val
31+
satisfies_predicate(.x, .p, ..., .purrr_predicate = "some")
5432
}
5533

5634
#' @export
5735
#' @rdname every
5836
none <- function(.x, .p, ...) {
59-
every(.x, negate(.p), ...)
37+
satisfies_predicate(.x, .p, ..., .purrr_predicate = "none")
38+
}
39+
40+
satisfies_predicate <- function(
41+
.x,
42+
.p,
43+
...,
44+
.purrr_predicate,
45+
.purrr_user_env = caller_env(2),
46+
.purrr_error_call = caller_env()
47+
) {
48+
# Not using `as_predicate()` as R level predicate result checks are too slow.
49+
# Checks are done at the C level instead (#1169). Also, `NA` propagates
50+
# through these functions, which `as_predicate()` doesn't allow.
51+
.p <- as_mapper(.p, ...)
52+
53+
# Consistent with `map()`
54+
.x <- vctrs_vec_compat(.x, .purrr_user_env)
55+
obj_check_vector(.x, arg = ".x", call = .purrr_error_call)
56+
57+
n <- vec_size(.x)
58+
59+
i <- 0L
60+
61+
# We refer to `.p`, `.x`, `i`, `...`, and `.purrr_error_call` all from C level
62+
switch(
63+
.purrr_predicate,
64+
every = .Call(every_impl, environment(), n, i),
65+
some = .Call(some_impl, environment(), n, i),
66+
none = .Call(none_impl, environment(), n, i),
67+
abort("Unreachable", .internal = TRUE)
68+
)
6069
}

‎R/utils.R‎

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,6 @@ as_predicate <- function(
6060
.fn,
6161
...,
6262
.mapper,
63-
.allow_na = FALSE,
6463
.purrr_error_call = caller_env(),
6564
.purrr_error_arg = caller_arg(.fn)
6665
) {
@@ -75,10 +74,6 @@ as_predicate <- function(
7574
out <- .fn(...)
7675

7776
if (!is_bool(out)) {
78-
if (is_na(out) && .allow_na) {
79-
# Always return a logical NA
80-
return(NA)
81-
}
8277
cli::cli_abort(
8378
"{.fn { .purrr_error_arg }} must return a single `TRUE` or `FALSE`, not {.obj_type_friendly {out}}.",
8479
arg = .purrr_error_arg,
@@ -115,8 +110,9 @@ vctrs_list_compat <- function(
115110
}
116111

117112
# When we want to use vctrs, but treat lists like purrr does
118-
# Treat data frames and S3 scalar lists like bare lists.
119-
# But ensure rcrd vctrs retain their class.
113+
#
114+
# Treats data frames and S3 scalar lists like bare lists.
115+
# But ensures rcrd vctrs retain their class.
120116
vctrs_vec_compat <- function(x, user_env) {
121117
if (inherits(x, "by")) {
122118
class(x) <- NULL
@@ -127,7 +123,7 @@ vctrs_vec_compat <- function(x, user_env) {
127123
} else if (is.pairlist(x)) {
128124
lifecycle::deprecate_soft(
129125
when = "1.0.0",
130-
what = I("Use of pairlists in map functions"),
126+
what = I("Use of pairlists in purrr functions"),
131127
details = "Please coerce explicitly with `as.list()`",
132128
user_env = user_env
133129
)
@@ -138,7 +134,7 @@ vctrs_vec_compat <- function(x, user_env) {
138134
} else if (is_call(x) || is.expression(x)) {
139135
lifecycle::deprecate_soft(
140136
when = "1.0.0",
141-
what = I("Use of calls and pairlists in map functions"),
137+
what = I("Use of calls and expressions in purrr functions"),
142138
details = "Please coerce explicitly with `as.list()`",
143139
user_env = user_env
144140
)

‎revdep/README.md‎

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
# Revdeps
22

3-
## Failed to check (24)
3+
## Failed to check (31)
44

55
|package |version |error |warning |note |
66
|:----------------|:-------|:------|:-------|:----|
77
|apa |? | | | |
88
|arealDB |0.9.4 |1 | | |
99
|ClusterGVis |? | | | |
1010
|dsTidyverse |? | | | |
11+
|eye |? | | | |
1112
|ggmosaic |0.3.3 |1 | | |
13+
|heplots |? | | | |
14+
|hmde |? | | | |
15+
|httk |? | | | |
1216
|immcp |? | | | |
17+
|imt |? | | | |
18+
|intervalpsych |? | | | |
19+
|irtQ |? | | | |
20+
|isotracer |? | | | |
21+
|jpcity |? | | | |
1322
|[kerastuneR](failures.md#kerastuner)|0.1.0.7 |__+1__ | | |
1423
|lcsm |? | | | |
1524
|metajam |0.3.1 |1 | | |
@@ -18,28 +27,25 @@
1827
|numbat |? | | | |
1928
|ontologics |0.7.4 |1 | | |
2029
|rdflib |0.2.9 |1 | | |
30+
|rmweather |? | | | |
31+
|rstanemax |? | | | |
2132
|RVA |? | | | |
33+
|sampler |? | | | |
34+
|sbm |? | | | |
2235
|SCpubr |? | | | |
23-
|sprtt |? | | | |
24-
|ssdGSA |? | | | |
25-
|[stoRy](failures.md#story)|0.2.2 |__+1__ | | |
2636
|Surrogate |? | | | |
27-
|tidybins |? | | | |
28-
|tidycomm |? | | | |
29-
|[tidyjson](failures.md#tidyjson)|0.3.2 |__+1__ | |-1 |
3037
|TriDimRegression |1.0.2 |1 | | |
3138

32-
## New problems (9)
39+
## New problems (8)
3340

34-
|package |version |error |warning |note |
35-
|:--------------|:-------|:--------|:-------|:----|
36-
|[autothresholdr](problems.md#autothresholdr)|1.4.2 |__+1__ | |1 |
37-
|[casino](problems.md#casino)|0.1.0 |__+1__ | |2 |
38-
|[CPAT](problems.md#cpat)|0.1.0 |__+1__ | |1 |
39-
|[egor](problems.md#egor)|1.24.2 |__+2__ | | |
40-
|[immunarch](problems.md#immunarch)|0.9.1 |__+1__ | |1 |
41-
|[moranajp](problems.md#moranajp)|0.9.7 |__+1__ | | |
42-
|[quincunx](problems.md#quincunx)|0.1.10 |__+2__ | | |
43-
|[SCORPIUS](problems.md#scorpius)|1.0.9 | |__+1__ |1 |
44-
|[tfrmt](problems.md#tfrmt)|0.2.0 |1 __+1__ | | |
41+
|package |version |error |warning |note |
42+
|:---------|:-------|:------|:-------|:--------|
43+
|[casino](problems.md#casino)|0.1.0 |__+1__ | |2 |
44+
|[CPAT](problems.md#cpat)|0.1.0 |__+1__ | |1 |
45+
|[egor](problems.md#egor)|1.24.2 |__+2__ | | |
46+
|[immunarch](problems.md#immunarch)|0.9.1 |__+1__ | |1 |
47+
|[meta](problems.md#meta)|8.2-1 | | |1 __+1__ |
48+
|[moranajp](problems.md#moranajp)|0.9.7 |__+1__ | | |
49+
|[quincunx](problems.md#quincunx)|0.1.10 |__+2__ | | |
50+
|[SCORPIUS](problems.md#scorpius)|1.0.9 | |__+1__ |1 |
4551

‎revdep/cran.md‎

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,15 @@
11
## revdepcheck results
22

3-
We checked 2125 reverse dependencies, comparing R CMD check results across CRAN and dev versions of this package.
3+
We checked 1702 reverse dependencies (1701 from CRAN + 1 from Bioconductor), comparing R CMD check results across CRAN and dev versions of this package.
44

5-
* We saw 9 new problems
6-
* We failed to check 24 packages
5+
* We saw 8 new problems
6+
* We failed to check 30 packages
77

88
Issues with CRAN packages are summarised below.
99

1010
### New problems
1111
(This reports the first line of each new failure)
1212

13-
* autothresholdr
14-
checking re-building of vignette outputs ... ERROR
15-
1613
* casino
1714
checking re-building of vignette outputs ... ERROR
1815

@@ -26,6 +23,9 @@ Issues with CRAN packages are summarised below.
2623
* immunarch
2724
checking examples ... ERROR
2825

26+
* meta
27+
checking installed package size ... NOTE
28+
2929
* moranajp
3030
checking examples ... ERROR
3131

@@ -36,17 +36,22 @@ Issues with CRAN packages are summarised below.
3636
* SCORPIUS
3737
checking whether package ‘SCORPIUS’ can be installed ... WARNING
3838

39-
* tfrmt
40-
checking examples ... ERROR
41-
4239
### Failed to check
4340

4441
* apa (NA)
4542
* arealDB (NA)
4643
* ClusterGVis (NA)
4744
* dsTidyverse (NA)
4845
* ggmosaic (NA)
46+
* heplots (NA)
47+
* hmde (NA)
48+
* httk (NA)
4949
* immcp (NA)
50+
* imt (NA)
51+
* intervalpsych (NA)
52+
* irtQ (NA)
53+
* isotracer (NA)
54+
* jpcity (NA)
5055
* kerastuneR (NA)
5156
* lcsm (NA)
5257
* metajam (NA)
@@ -55,13 +60,11 @@ Issues with CRAN packages are summarised below.
5560
* numbat (NA)
5661
* ontologics (NA)
5762
* rdflib (NA)
63+
* rmweather (NA)
64+
* rstanemax (NA)
5865
* RVA (NA)
66+
* sampler (NA)
67+
* sbm (NA)
5968
* SCpubr (NA)
60-
* sprtt (NA)
61-
* ssdGSA (NA)
62-
* stoRy (NA)
6369
* Surrogate (NA)
64-
* tidybins (NA)
65-
* tidycomm (NA)
66-
* tidyjson (NA)
6770
* TriDimRegression (NA)

0 commit comments

Comments
 (0)