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

[BUG] galah_select(group = "assertions") causes error #137

Closed
stewartmacdonald opened this issue Feb 22, 2022 · 3 comments
Closed

[BUG] galah_select(group = "assertions") causes error #137

stewartmacdonald opened this issue Feb 22, 2022 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@stewartmacdonald
Copy link

stewartmacdonald commented Feb 22, 2022

Describe the bug
In an effort to debug this issue, I seem to have stuffed something up because I can no longer download any records with galah. Thus, this bug report is going from memory.

Using the example code works fine:

result <- galah_call() |>
	galah_identify("Litoria") |>
	galah_filter(year >= 2020, cl22 == "Tasmania") |>
	galah_select(basisOfRecord, group = "basic") |>
	atlas_occurrences()

But changing this to group = 'assertions' causes an error about the $ operator being invalid for vectors.

Galah 1.4.0

To Reproduce
Steps to reproduce the behaviour:

> library('galah')
> galah_config(email = "email@example.com")

> result <- galah_call() |>
	galah_identify("Litoria") |>
	galah_filter(year >= 2020, cl22 == "Tasmania") |>
	galah_select(basisOfRecord, group = "assertions") |>
	atlas_occurrences()
Error: $ operator is invalid for atomic vectors
In addition: Warning message:
We didn't detect a field to search for.
ℹ Try entering text to search for matching fields.
ℹ To see all valid fields, use `show_all_fields()`. 

Expected behaviour
I expect to get a list of records along with their associated assertion data.

Additional context
This bug appears to come from the preset_cols() helper function in galah_select.R. Changing this line:

"assertions" = search_fields(type = "assertions")$id)

to:

"assertions" = show_all_fields(type = "assertions")$id)

seems to fix the issue. I have submitted pull request #135 that fixes this bug and expands on the documentation, but I have no idea what I'm doing.

@stewartmacdonald stewartmacdonald added the bug Something isn't working label Feb 22, 2022
mjwestgate added a commit that referenced this issue Feb 23, 2022
- fix bug where `group = "assertions"` failed (#137)
- fix bug where `group = "basic"` columns were always included (#128)
- update behaviour of validate fields to recognise `show_all_assertions` (i.e. assertions no longer included in `show_all_fields`)
- add tests to prevent recurrences (all passing)
@shandiya shandiya self-assigned this Jul 4, 2022
@daxkellie daxkellie reopened this Oct 31, 2022
@daxkellie
Copy link
Collaborator

It looks like there is still an issue with downloads when using galah_select(group = "assertions")

As an example, this query returns an error:

library(galah)
library(magrittr)

galah_config(email = "your_email_here")

galah_call() %>% 
  galah_identify("animalia") %>% 
  galah_identify("https://biodiversity.org.au/afd/taxa/3cbb537e-ab39-4d85-864e-76cd6b6d6572", search = FALSE) %>% 
  galah_filter(basisOfRecord == "PRESERVED_SPECIMEN", 
               year == 2022) %>%  # Limiting to 2022 for now
  galah_select(group = "assertions") %>%  
  atlas_occurrences() 
#> Calling the API failed for `atlas_occurrences`.
#> ℹ This might mean that the selected system is down. Double check that your query is correct.
#> ℹ If you continue to see this message, please email support@ala.org.au.
#> # A tibble: 0 × 0

Created on 2022-10-31 with reprex v2.0.2

@mjwestgate
Copy link
Collaborator

This looks like a bug in the code for passing assertions to atlas_occurrences, or specifically, inside the subfunction build_assertion_columns (stored in utilities_internal.R), which looks like this:

build_assertion_columns <- function(col_df) {
  if (nrow(col_df) == 0) {
    return("none")
    # all assertions have been selected
  } else if (nrow(col_df) == 107) {
    return("includeall")
  }
  paste0(col_df$name, collapse = ",")
}

This function checks whether ‘all’ assertions are requested by testing whether 107 rows have been passed; but since that was written, the ALA has updated their assertions, and there are now 116. Therefore atlas_occurrences concatenates all assertion names into the field arg, instead of simply passing &qa=includeall, which is the preferred solution.

A better solution is to record which groups are passed by galah_select (possibly using call attribute?) and detecting that within atlas_occurrences.

@mjwestgate mjwestgate self-assigned this Nov 28, 2022
@mjwestgate
Copy link
Collaborator

fixed as of version 1.5.1

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

4 participants