Merged
Conversation
This reverts commit 810e40c.
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the ChEMBL query interface to support both webservice and offline database modes. The key changes include introducing a chembl_options() function for configuration parameters, splitting the query logic into separate chembl_query_ws() and chembl_query_offline() functions, and updating the test parameter test_service_down to be passed via ... arguments instead of as an explicit parameter.
Key changes:
- Added
chembl_options()function to consolidate query configuration options (cache_file, similarity, tidy, version) - Refactored
chembl_query()to support both "ws" (webservice) and "offline" modes via a newmodeparameter - Created new file R/chembl_offline.R with offline query implementations for various ChEMBL resources
- Updated function signatures across multiple functions to use
...for internal testing arguments
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 35 comments.
Show a summary per file
| File | Description |
|---|---|
| R/chembl.R | Added chembl_options() helper, refactored chembl_query() to support mode selection, updated test parameter handling in multiple functions |
| R/chembl_offline.R | New file implementing offline query functionality with resource-specific handlers and comparison utilities |
| tests/testthat/test-chembl.R | Updated test calls to use chembl_options() syntax and improved code formatting |
| man/chembl_query.Rd | Updated documentation to reflect new API design with mode parameter and options function |
| man/chembl_status.Rd | Updated to document ... parameter instead of test_service_down |
| man/chembl_img.Rd | Updated to document ... parameter instead of test_service_down |
| man/chembl_atc_classes.Rd | Updated to document ... parameter instead of test_service_down |
Comments suppressed due to low confidence (1)
R/chembl.R:1
- Example shows passing a bare
list()to theoptionsparameter, but the function signature and parameter documentation indicateoptionsshould be the result of callingchembl_options(). The example should useoptions = chembl_options(tidy = FALSE)for consistency with the function's design, or clarify that both forms are acceptable.
#' Control options for ChEMBL queries
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR expands the functionality of
chembl_query(). It adds a new argumentmodewhich can be either "ws" (default) or "offline". This governs whether the function will attempt to retrieve data from the webservice or extract from the offline database. Note, ChEMBL contains almost 30 resources. For this PR I focused on establishing the framework for implementing offline access. Currenly only the "molecule" resource is supported for offline queries, the others will return an informative error.Other then improving this user facing function I have added other functions as well which help the dev work.
PR task list:
devtools::document()