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

No option exists to check whether a value exists within a specified field #131

Closed
mjwestgate opened this issue Feb 2, 2022 · 4 comments
Closed
Assignees
Labels
enhancement New feature or request

Comments

@mjwestgate
Copy link
Collaborator

In v.1.4.0, we have show_all_fields() to show which fields are valid, and search_fields() to run a search within show_all_fields (via grepl). This is also used internally when galah_config(run_checks = TRUE) to ensure that user-supplied field names exist in the specified atlas. However, there is no equivalent for values within a field.

Specifically, search_field_values() is conceptually closer to show_all_fields than search_fields, because it returns all possible values for the specified field. Then the user must build their own search using the resulting tibble to check if the specified value is present.

Potentially improved behavior is:

  • show_all_values() with a mandatory field argument, basically the same as the current search_field_values
  • search_values() with arguments field and string, to allow searching within a specified field

A further benefit of this approach is that we could support value checking when galah_config(run_checks = TRUE) using show_all_values, in the same way as we currently use show_all_fields.

As an aside, default behaviour for atlas_counts is that setting limit = NULL returns all values, but this doesn't work for search_field_values.

@mjwestgate mjwestgate added the enhancement New feature or request label Feb 2, 2022
@mjwestgate
Copy link
Collaborator Author

Decision 2022-02-08 to action this, with a caveat that the whole search_ and show_all_ syntax is a little clumsy and may need further thought.

mjwestgate added a commit that referenced this issue Feb 8, 2022
Slightly messy code, but should show proof of concept
@mjwestgate mjwestgate self-assigned this Jul 20, 2022
@mjwestgate
Copy link
Collaborator Author

mjwestgate commented Aug 31, 2022

On reflection, there is a further problem here. show_all_values is actually nested wishing show_all_fields, because show_all_values requires a field argument to return a sensible result. But this nestedness is obscured by giving both functions the same prefix. A similar problem occurs with show_all_profiles and search_profile_attributes. Species lists aren't implemented yet, but have the same issue again.

A better choice would be to separate out these 'nested' lookup functions into a new set of functions. Preliminary choice is:

  • show_field_values (replacement for show_all_values)
  • show_profile_values (replacement for search_profile_attributes)
  • show_list_values (new function for species lists)

mjwestgate added a commit that referenced this issue Aug 31, 2022
- new function `show_values` to show subsets from `show_all`; deprecate `show_all_values` (#131)
- rename `show_all_species_lists` to `show_all_lists`
- new function `show_all_apis` (#126)
- replace `search_profile_attributes` with `show_profile_values`
@daxkellie
Copy link
Contributor

Further discussion has led to a possible solution of show_values() and search_values(), with the above functions set as internal to to these 2 values look-up functions. These can be piped from a search result of an accepted information type. For example:

search_all(fields, "cl22") |> show_values()
search_all(fields, "cl22") |> search_values("tasmania")

daxkellie added a commit that referenced this issue Sep 30, 2022
* Make `field`, `profile` and `list` values functions internal
* Improve error messages in `search_` functions
daxkellie added a commit that referenced this issue Oct 7, 2022
* Update deprecated values functions & warnings
* `show_values` and `search_values` tests added
* Internal `search_list_values` & `search_profile_values` added
@mjwestgate
Copy link
Collaborator Author

fixed with addition of show_values and search_values

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants