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

x[1,,drop = TRUE] should return a list #367

Closed
patperry opened this Issue Jan 17, 2018 · 6 comments

Comments

Projects
2 participants
@patperry
Contributor

patperry commented Jan 17, 2018

Currently tibble behavior with drop = TRUE is consistent with data.frame for extracting a single column, but not for extracting a single row:

library(tibble)
as_tibble(mtcars)[1, , drop = TRUE]
#> # A tibble: 1 x 11
#>     mpg   cyl  disp    hp  drat    wt  qsec    vs    am  gear  carb
#>   <dbl> <dbl> <dbl> <dbl> <dbl> <dbl> <dbl> <dbl> <dbl> <dbl> <dbl>
#> 1  21.0  6.00   160   110  3.90  2.62  16.5     0  1.00  4.00  4.00
mtcars[1, , drop = TRUE]
#> $mpg
#> [1] 21
#> 
#> $cyl
#> [1] 6
#> 
#> $disp
#> [1] 160
#> 
#> $hp
#> [1] 110
#> 
#> $drat
#> [1] 3.9
#> 
#> $wt
#> [1] 2.62
#> 
#> $qsec
#> [1] 16.46
#> 
#> $vs
#> [1] 0
#> 
#> $am
#> [1] 1
#> 
#> $gear
#> [1] 4
#> 
#> $carb
#> [1] 4

The data.frame behavior makes more sense to me here, because it fits with the general pattern that if x is a matrix-like object, then x[1,,drop = TRUE] returns a vector-like object.

Related: #311

@krlmlr krlmlr added the bug label Jan 17, 2018

@krlmlr

This comment has been minimized.

Member

krlmlr commented Jan 17, 2018

Thanks, agreed. We're supporting the drop argument for consistency with packages that expect data frames, and we should also support this case.

@patperry

This comment has been minimized.

Contributor

patperry commented Jan 17, 2018

I think you have some leeway here for what to do for the following cases:

x <- mtcars[1, ]
x[, , drop = TRUE]

y <- mtcars[, 1, drop = FALSE]
y[, , drop = TRUE]

z <- mtcars[1, 1, drop = FALSE]
z[, , drop = TRUE]

A data.frame returns a list in the first case and a vector in the second and third.

In my own implementation in the frame package, (https://github.com/patperry/r-frame ) I chose to break with the data.frame behavior and only drop dimensions with non-missing indices. This makes the results have the same type in all three cases above (x[, , drop = TRUE] returns x, with similar behavior for y and z).

Probably no one actually uses this construct, so do whatever makes the implementation easiest.

@patperry

This comment has been minimized.

Contributor

patperry commented Jan 17, 2018

Following up, this is the corner case that actually matters:

x <- data.frame(foo = 31337)
x[1, , drop = TRUE]

A data.frame returns a 31337 here, but I think it's more natural to return list(foo = 31337).

@krlmlr

This comment has been minimized.

Member

krlmlr commented Jan 17, 2018

I think users should always use drop = FALSE. We're not going to change drop = TRUE behavior for tibble, this is really only for data.frame compatibility

@krlmlr krlmlr added this to To Do in krlmlr Jan 18, 2018

@krlmlr

This comment has been minimized.

Member

krlmlr commented Jan 20, 2018

This seems to only require splitting the attributes from the resulting list (including "class").

krlmlr added a commit that referenced this issue Jan 20, 2018

fix corner case of selecting one row with drop = TRUE, #367
- `tbl[1, , drop = TRUE]` now behaves identically to data frames (#367).
@krlmlr

This comment has been minimized.

Member

krlmlr commented Jan 20, 2018

Closed in 35dfee5.

@krlmlr krlmlr closed this Jan 20, 2018

krlmlr automation moved this from To Do to Done Jan 20, 2018

krlmlr added a commit that referenced this issue Jan 20, 2018

Merge tag 'v1.4.1.9001'
- `enframe(NULL)` now returns the same as `enframe(logical())` (#352).
- `tbl[1, , drop = TRUE]` now behaves identically to data frames (#367).
- The `tibble.width` option is honored again (#369).
- Faster printing of very wide tibbles (#360).
- Update vignette to match changes in 1.4.1 (#368, @bgreenwell).
- Don't rely on `ncol()` for `glimpse()`, only query `nrow()` and `head()`.
- Return input for zero-column data frames.
- Add test for `glimpse()` with unknown rows (#366, @kevinykuo).
- Faster construction and subsetting for tibbles (#353).
- `tribble()` now ignores trailing commas (#342, @LaDilettante).
- Fix error message when accessing columns using a logical index vector (#337, @mundl).

krlmlr added a commit that referenced this issue Jan 23, 2018

Merge tag 'v1.4.2'
Bug fixes
---------

- Fix OS X builds.
- The `tibble.width` option is honored again (#369).
- `tbl[1, , drop = TRUE]` now behaves identically to data frames (#367).
- Fix error message when accessing columns using a logical index vector (#337, @mundl).
- `glimpse()` returns its input for zero-column data frames.

Features
--------

- `enframe(NULL)` now returns the same as `enframe(logical())` (#352).
- `tribble()` now ignores trailing commas (#342, @LaDilettante).
- Updated vignettes and website documentation.

Performance
-----------

- Faster printing of very wide tibbles (#360).
- Faster construction and subsetting for tibbles (#353).
- Only call `nrow()` and `head()` in `glimpse()`, not `ncol()`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment