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

Possible reversion on dim()/ncol()? #976

Closed
JohnMount opened this Issue Aug 30, 2017 · 4 comments

Comments

Projects
None yet
3 participants
@JohnMount

JohnMount commented Aug 30, 2017

It looks like none of dim()/ncol()/nrow() are currently returning non-NA values on the current version of sparklyr. I get that counting rows is expensive (and you may not want to do that) but it seems like the system had fewer NAs when this was addressed in issue 346.

suppressPackageStartupMessages(library("dplyr"))
library("sparklyr")
packageVersion("dplyr")
#> [1] '0.7.2.9000'
packageVersion("sparklyr")
#> [1] '0.6.2'
packageVersion("dbplyr")
#> [1] '1.1.0.9000'

sc <- spark_connect(master = 'local')
#> * Using Spark: 2.1.0
d <- dplyr::copy_to(sc, data.frame(x = 1:2))
dim(d)
#> [1] NA
ncol(d)
#> [1] NA
nrow(d)
#> [1] NA


spark_disconnect(sc)
@kevinykuo

This comment has been minimized.

Show comment
Hide comment
@kevinykuo

kevinykuo Aug 30, 2017

Collaborator

See #890

Collaborator

kevinykuo commented Aug 30, 2017

See #890

@kevinykuo

This comment has been minimized.

Show comment
Hide comment
@kevinykuo

kevinykuo Sep 1, 2017

Collaborator

Thanks for the writeup at http://www.win-vector.com/blog/2017/08/why-to-use-the-replyr-r-package/. Leaving this open so we can take a deeper look before the next release; I think at a minimum we should have a message on why these functions don't work if nothing else.

Collaborator

kevinykuo commented Sep 1, 2017

Thanks for the writeup at http://www.win-vector.com/blog/2017/08/why-to-use-the-replyr-r-package/. Leaving this open so we can take a deeper look before the next release; I think at a minimum we should have a message on why these functions don't work if nothing else.

@JohnMount

This comment has been minimized.

Show comment
Hide comment
@JohnMount

JohnMount Sep 1, 2017

@kevinykuo Thanks for all you do on this project- I have been impressed. I do understand it is not my decision.

JohnMount commented Sep 1, 2017

@kevinykuo Thanks for all you do on this project- I have been impressed. I do understand it is not my decision.

@kevinykuo

This comment has been minimized.

Show comment
Hide comment
@kevinykuo

kevinykuo Jan 17, 2018

Collaborator

Closing this one. Now we have nrow() returning NA (NA_real_) and ncol() returning a value to be consistent with other backends. Note that ncol() can still be expensive.

Collaborator

kevinykuo commented Jan 17, 2018

Closing this one. Now we have nrow() returning NA (NA_real_) and ncol() returning a value to be consistent with other backends. Note that ncol() can still be expensive.

@kevinykuo kevinykuo closed this Jan 17, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment