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

partial evaluation of geom predicates only goes one level deep #146

Closed
ateucher opened this issue Nov 27, 2019 · 6 comments
Closed

partial evaluation of geom predicates only goes one level deep #146

ateucher opened this issue Nov 27, 2019 · 6 comments
Assignees
Labels
bug Something isn't working
Milestone

Comments

@ateucher
Copy link
Collaborator

library(bcdata)
library(sf)
#> Linking to GEOS 3.7.2, GDAL 2.4.2, PROJ 5.2.0

crd_mun <- bcdc_query_geodata("municipalities-legally-defined-administrative-areas-of-bc") %>% 
  select(ADMIN_AREA_ABBREVIATION, ADMIN_AREA_GROUP_NAME) %>% 
  filter(ADMIN_AREA_GROUP_NAME == "Capital Regional District") %>% 
  collect()

# Doesn't work
crd_greenspaces <- bcdc_query_geodata("local-and-regional-greenspaces") %>% 
  select(PARK_NAME, PARK_TYPE, PARK_PRIMARY_USE) %>% 
  filter(BBOX(st_bbox(crd_mun), crs = "EPSG:3005")) %>% #<<
  collect()
#> Error: Cannot embed a data frame in a SQL query.
#> 
#> If you are seeing this error in code that used to work, the most likely
#> cause is a change dbplyr 1.4.0. Previously `df$x` or `df[[y]]` implied
#> that `df` was a local variable, but now you must make that explict with
#> `!!` or `local()`, e.g., `!!df$x` or `local(df[["y"]))

the_box <- st_bbox(crd_mun)
# works
crd_greenspaces <- bcdc_query_geodata("local-and-regional-greenspaces") %>% 
  select(PARK_NAME, PARK_TYPE, PARK_PRIMARY_USE) %>% 
  filter(BBOX(the_box, crs = "EPSG:3005")) %>% #<<
  collect()

Created on 2019-11-27 by the reprex package (v0.3.0)

@ateucher ateucher added the bug Something isn't working label Nov 27, 2019
@ateucher ateucher added this to the 0.1.2 milestone Nov 27, 2019
@ateucher ateucher changed the title partial evaluation of geom_predicates only goes one level deep partial evaluation of geom predicates only goes one level deep Nov 27, 2019
@ateucher ateucher self-assigned this Dec 9, 2019
@ateucher
Copy link
Collaborator Author

ateucher commented Dec 9, 2019

The problem seems to be not the nesting, but that all functions are assumed to be remote. From the examples in?dbplyr::partial_eval:

# Functions are always assumed to be remote. Use local to force evaluation in R.
f <- function(x) x + 1
partial_eval(quote(year > f(1980)), vars = vars)
partial_eval(quote(year > local(f(1980))), vars = vars)

So this does work:

crd_greenspaces <- bcdc_query_geodata("local-and-regional-greenspaces") %>% 
  filter(BBOX(local(st_bbox(crd_mun)), crs = "EPSG:3005")) %>% #<<
  collect()

@ateucher
Copy link
Collaborator Author

ateucher commented Dec 9, 2019

I don't know if we can insert a local() for any function that's not one of the geom predicates? Or make an informative error message/help section telling users to use local()...

@ateucher
Copy link
Collaborator Author

ateucher commented Dec 9, 2019

Also works (using !! instead of local()):

crd_greenspaces <- bcdc_query_geodata("local-and-regional-greenspaces") %>% 
  filter(BBOX(!!st_bbox(crd_mun), crs = "EPSG:3005")) %>% #<<
  collect()

@boshek
Copy link
Collaborator

boshek commented Dec 9, 2019

In that sense the error you receive is actually quite good. Other than this part:

Error: Cannot embed a data frame in a SQL query.

and this part:

the most likely cause is a change dbplyr 1.4.0

So tweaking the error message and including an example should be sufficient?

@ateucher
Copy link
Collaborator Author

ateucher commented Dec 9, 2019

Yeah, the tricky part is going to be catching those in a general way so that we know when to throw that error...

@boshek
Copy link
Collaborator

boshek commented Dec 9, 2019

Can you just catch the dbplyr error and assume that dbplyr will catch all those cases?

ateucher added a commit that referenced this issue Dec 11, 2019
ateucher added a commit that referenced this issue Dec 12, 2019
Instead of early evaluation of nested calls as previously attempted, modify the call to wrap nested calls in local() so that dbplyr::partial_eval knows to evaluate them. #146
@boshek boshek closed this as completed in 2dd503e Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants