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

OR statement not working in galah_filter() #169

Closed
daxkellie opened this issue Nov 3, 2022 · 2 comments
Closed

OR statement not working in galah_filter() #169

daxkellie opened this issue Nov 3, 2022 · 2 comments
Assignees

Comments

@daxkellie
Copy link
Contributor

galah_filter() does not return the second half of an OR statement in a query

library(galah)

galah_call() |>
  galah_filter(phylum == "Chordata") |>
  atlas_counts()
#> # A tibble: 1 × 1
#>      count
#>      <int>
#> 1 77396036

galah_call() |>
  galah_filter(kingdom == "Plantae") |>
  atlas_counts()
#> # A tibble: 1 × 1
#>      count
#>      <int>
#> 1 23384918

galah_call() |>
  galah_filter(phylum == "Chordata" | kingdom == "Plantae") |>
  atlas_counts()
#> # A tibble: 1 × 1
#>      count
#>      <int>
#> 1 77396036

galah_call() |>
  galah_filter(kingdom == "Plantae" | phylum == "Chordata") |>
  atlas_counts()
#> # A tibble: 1 × 1
#>      count
#>      <int>
#> 1 23384918

Created on 2022-11-03 with reprex v2.0.2

/cc @mjwestgate

@mjwestgate
Copy link
Collaborator

Looking in detail here, it appears that galah_filter is ignoring the second field name:

galah_filter(phylum == "Chordata" | kingdom == "Plantae")
# A tibble: 1 × 4
  variable logical value                                  query                                        
  <chr>    <chr>   <chr>                                  <chr>                                        
1 phylum   ==      "c(\\\"Chordata\\\", \\\"Plantae\\\")" "(phylum:\"Chordata\" OR phylum:\"Plantae\")"

I'll look at the parsing logic now

@mjwestgate mjwestgate self-assigned this Apr 28, 2023
mjwestgate added a commit that referenced this issue Apr 28, 2023
Currently incomplete, but re-architected, version of `filter` to rely less on strings and more on `rlang`. Solves problems with incorrect parsing of OR statements (#154 & #169). Output same format as previously, but much faster. New method is currently only available via `filter`, not `galah_filter`.
mjwestgate added a commit that referenced this issue Jun 9, 2023
- new function `parse_quosures_basic` for capturing NSE in `group_by`, `select` etc (#154, #169)
- ensure `dplyr` and `galah` versions of same functions are in same files, and share help files
- remove remaining cache functions
- add `collapse`, `compute` and `collect` help file (#183)
daxkellie added a commit that referenced this issue Jun 15, 2023
* `galah_filter()` didn't build the second half of `OR` statements correctly
* Added `rlang::expr_text()` to handle expr parsing
* `galah_filter()` did not support queries with blanks (e.g. eventDate == "")
* Code readded from older version to handle queries with blanks
* Fixes parsing of `is.na`
@daxkellie
Copy link
Contributor Author

Fixed piping issue in new galah_filter parsing logic in commit a0b2a3c (#154, #169)

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