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

fnrow() returns error for 0-column data frames #344

Closed
NicChr opened this issue Dec 22, 2022 · 3 comments
Closed

fnrow() returns error for 0-column data frames #344

NicChr opened this issue Dec 22, 2022 · 3 comments

Comments

@NicChr
Copy link

NicChr commented Dec 22, 2022

Hi, I noticed that fnrow() throws an out of bounds error when called on a data frame with 0 columns.
See an example below.

> library(collapse)
collapse 1.9.0.9000, see ?`collapse-package` or ?`collapse-documentation`

Attaching package: ‘collapse’

The following object is masked from ‘package:stats’:

    D

data(iris)
iris2 <- iris[, NULL, drop = FALSE]
iris3 <- iris[NULL, , drop = FALSE]
iris4 <- iris[NULL, NULL, drop = FALSE]
nrow(iris2)
[1] 150
ncol(iris2)
[1] 0
nrow(iris3)
[1] 0
ncol(iris3)
[1] 5
nrow(iris4)
[1] 0
ncol(iris4)
[1] 0
fnrow(iris2)
Error in .subset2(X, 1L) : subscript out of bounds
fncol(iris2)
[1] 0
fnrow(iris3)
[1] 0
fncol(iris3)
[1] 5
fnrow(iris4)
Error in .subset2(X, 1L) : subscript out of bounds
fncol(iris4)
[1] 0

Many thanks

@SebKrantz
Copy link
Owner

Thanks, I however don't see how this could be solved efficiently in R. The function is currently defined as

fnrow <- function (X) if (is.list(X)) length(.subset2(X, 1L)) else dim(X)[1L]

so the error comes from base:::.subset2 which fetches the first column. I could of course do an additional check length(X) > 0 or something like that, but the issue is that length(X) has some overhead on a data.frame. Another option would be rewriting the function in C, but this would incur some .Call overhead. I'll think about the C rewrite, although it would be a bit annoying for such a small function. For the time being I recommend to just use nrow() in cases where the data frame could have zero rows.

@NicChr
Copy link
Author

NicChr commented Dec 23, 2022

Thanks, I would add that I think it affects other functions such as seq_row() and fsubset() for the special case of 0-column data frames.

Would something like this maybe work?

fnrow <- function (X){
    if (inherits(X, "data.frame")){
        length(attr(X, "row.names", exact = TRUE))
    } else {
        NULL
    }
}

You can maybe use the fact that data.frames have a row.names attribute though it's a bit slower than collapse::fnrow() but still faster than nrow().

@SebKrantz
Copy link
Owner

Thanks. I have rewritten fnrow in C. It is in the main branch now.

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

2 participants