Skip to content

Commit f4c8f77

Browse files
authored
Fix vec_order_info() when there are zero columns (#1866)
* Push a group size in the 0 column early exit case * NEWS bullet
1 parent e88a3e2 commit f4c8f77

File tree

4 files changed

+51
-7
lines changed

4 files changed

+51
-7
lines changed

NEWS.md

Lines changed: 9 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
# vctrs (development version)
22

3+
* Fixed an issue with `vec_rank()` and 0-column data frames (#1863).
4+
35
# vctrs 0.6.3
46

57
* Fixed an issue where certain ALTREP row names were being materialized when
@@ -56,7 +58,7 @@
5658
`multiple = "warning"`, which have been removed from the documentation and
5759
silently soft-deprecated. Official deprecation for those options will start in
5860
a future release (#1791).
59-
61+
6062
* `vec_locate_matches()` has changed its default `needles_arg` and
6163
`haystack_arg` values from `""` to `"needles"` and `"haystack"`, respectively.
6264
This generally generates more informative error messages (#1792).
@@ -70,7 +72,7 @@
7072

7173
* The `numeric_version` type from base R is now better supported in equality,
7274
comparison, and order based operations (tidyverse/dplyr#6680).
73-
75+
7476
* R >=3.5.0 is now explicitly required. This is in line with the tidyverse
7577
policy of supporting the [5 most recent versions of
7678
R](https://www.tidyverse.org/blog/2019/04/r-version-support/).
@@ -79,7 +81,7 @@
7981

8082
* New `vec_expand_grid()`, which is a lower level helper that is similar to
8183
`tidyr::expand_grid()` (#1325).
82-
84+
8385
* New `vec_set_intersect()`, `vec_set_difference()`, `vec_set_union()`, and
8486
`vec_set_symmetric_difference()` which compute set operations like
8587
`intersect()`, `setdiff()`, and `union()`, but the vctrs variants don't strip
@@ -146,7 +148,7 @@
146148
like specifying `repair = "unique", quiet = TRUE`. When the `"*_quiet"`
147149
options are used, any setting of `quiet` is silently overridden (@jennybc,
148150
#1629).
149-
151+
150152
`"unique_quiet"` and `"universal_quiet"` are also newly accepted for the name
151153
repair argument of several other functions that do not expose a `quiet`
152154
argument: `data_frame()`, `df_list()`, `vec_c()`, `list_unchop()`,
@@ -464,11 +466,11 @@
464466
to implement, but if your class has a static prototype, you might consider
465467
implementing a custom `vec_ptype()` method that returns a constant to
466468
improve performance in some cases (such as common type imputation).
467-
469+
468470
* New `vec_detect_complete()`, inspired by `stats::complete.cases()`. For most
469471
vectors, this is identical to `!vec_equal_na()`. For data frames and
470472
matrices, this detects rows that only contain non-missing values.
471-
473+
472474
* `vec_order()` can now order complex vectors (#1330).
473475

474476
* Removed dependency on digest in favor of `rlang::hash()`.
@@ -477,7 +479,7 @@
477479
when used as a data frame column (#1318).
478480

479481
* `register_s3()` is now licensed with the "unlicense" which makes it very
480-
clear that it's fine to copy and paste into your own package
482+
clear that it's fine to copy and paste into your own package
481483
(@maxheld83, #1254).
482484

483485
# vctrs 0.3.6

src/order.c

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3971,6 +3971,9 @@ void df_order_internal(SEXP x,
39713971
// Special case no columns
39723972
if (n_cols == 0) {
39733973
init_order(p_order);
3974+
if (size != 0) {
3975+
groups_size_maybe_push(size, p_group_infos);
3976+
}
39743977
return;
39753978
}
39763979

tests/testthat/test-order.R

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1275,6 +1275,29 @@ test_that("Indistinct NA and NaN are reported in the same group", {
12751275
expect_identical(info[[3]], 2L)
12761276
})
12771277

1278+
# ------------------------------------------------------------------------------
1279+
# `vec_order_info(<data.frame>)`
1280+
1281+
test_that("Zero column data frames with >0 rows work (#1863)", {
1282+
# All rows are treated as being from the same group
1283+
x <- data_frame(.size = 5)
1284+
info <- vec_order_info(x)
1285+
1286+
expect_identical(info[[1]], 1:5) # Order
1287+
expect_identical(info[[2]], 5L) # Group sizes
1288+
expect_identical(info[[3]], 5L) # Max group size
1289+
})
1290+
1291+
test_that("Zero column data frames with exactly 0 rows work (#1863)", {
1292+
# This is a particularly special case, since we don't actually push a group size
1293+
x <- data_frame(.size = 0L)
1294+
info <- vec_order_info(x)
1295+
1296+
expect_identical(info[[1]], integer())
1297+
expect_identical(info[[2]], integer())
1298+
expect_identical(info[[3]], 0L)
1299+
})
1300+
12781301
# ------------------------------------------------------------------------------
12791302
# vec_sort
12801303

tests/testthat/test-rank.R

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,22 @@ test_that("works with data frames", {
6363
expect_identical(vec_rank(df, ties = "sequential"), c(2L, 3L, 1L, 4L, 5L))
6464
})
6565

66+
test_that("works with data frames with 0 columns and >0 rows (#1863)", {
67+
# All rows are treated as being from the same group
68+
df <- data_frame(.size = 5)
69+
70+
expect_identical(vec_rank(df, ties = "min"), c(1L, 1L, 1L, 1L, 1L))
71+
expect_identical(vec_rank(df, ties = "sequential"), c(1L, 2L, 3L, 4L, 5L))
72+
expect_identical(vec_rank(df, ties = "sequential", direction = "desc"), c(1L, 2L, 3L, 4L, 5L))
73+
})
74+
75+
test_that("works with data frames with 0 columns and 0 rows (#1863)", {
76+
df <- data_frame(.size = 0)
77+
78+
expect_identical(vec_rank(df, ties = "min"), integer())
79+
expect_identical(vec_rank(df, ties = "sequential"), integer())
80+
})
81+
6682
test_that("can control the direction per column", {
6783
df <- data_frame(
6884
x = c(1, 2, 1, 2, 2),

0 commit comments

Comments
 (0)