v1.9.0 prep - #10
Conversation
- Updated package version to 1.9.0. - Introduced pagination for the `client$mappings$get()` method, allowing retrieval of multiple pages of mappings for a concept. - Added `client$mappings$get_all()` method to fetch all mappings across pages, ensuring complete data retrieval. - Adjusted documentation to reflect new pagination parameters and methods. - Enhanced tests to validate pagination functionality and ensure backward compatibility with existing calls.
- Changed the default value of `include_invalid` from `FALSE` to `NULL`, allowing the server's default behavior to be used. - Updated documentation to clarify the tri-state nature of `include_invalid`, specifying how to include or exclude deprecated mappings. - Enhanced tests to validate the new behavior of `include_invalid`, ensuring correct handling of requests with different parameter values.
- Introduced a new `relationship_ids` parameter in `client$mappings$get()` and `get_all()` methods, allowing users to specify relationship types to return. - Updated documentation to reflect the new parameter and its default behavior, clarifying how to retrieve composite concept decompositions. - Enhanced tests to validate the correct handling of `relationship_ids`, ensuring proper query formation and error handling for non-character inputs.
- Updated the README and example scripts to clarify the usage of `client$mappings$get()` and `client$mappings$get_all()`, emphasizing pagination and the importance of the `relationship_ids` parameter. - Improved explanations regarding the mapping direction and how to retrieve mappings for non-standard concepts. - Added examples demonstrating the retrieval of all mappings, including handling of deprecated mappings and composite concepts. - Adjusted output formatting in examples for better clarity and user understanding.
- Enhanced comments in `map_between_vocabularies.R` to explain the structure of mapping rows, emphasizing that vocabulary id and concept code are not included. - Updated output formatting to show resolved target concept details, including vocabulary id and concept code, for improved clarity in example usage.
There was a problem hiding this comment.
2 issues found across 8 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="R/mappings.R">
<violation number="1" location="R/mappings.R:46">
P2: Existing positional callers of `mappings$get()` now send old arguments to the wrong parameters and can fail validation instead of returning mappings. Keeping `include_invalid` and `vocab_release` in their original positions and appending the new arguments preserves the public method contract.</violation>
<violation number="2" location="R/mappings.R:119">
P3: `get()` and `get_all()` now carry two large, near-identical blocks of parameter documentation (the multi-paragraph `relationship_ids` and tri-state `include_invalid` descriptions). This duplication will drift if the endpoint semantics change (as they just did for pagination), since both copies must be updated in lockstep. Consider documenting these once and inheriting them on the second method with roxygen's `@inheritParams` to keep the contract consistent.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
…n updates - Added `relationship_ids` and `vocab_release` parameters to `client$mappings$get()` and `get_all()` methods, allowing users to specify relationship types and vocabulary release versions. - Updated documentation to clarify the behavior of the new parameters and their default values, ensuring users understand how to retrieve mappings effectively. - Adjusted tests to validate the correct handling of the new parameters, ensuring backward compatibility with existing positional calls.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
All reported issues were addressed across 3 files (changes from recent commits).
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
- Added `vocab_release` parameter to `client$mappings$get()` and `get_all()` methods, clarifying its usage in the documentation. - Adjusted the order of documented parameters to match method signatures, ensuring consistency and reducing potential confusion for users. - Enhanced tests to verify that the documented parameter order aligns with the actual method signatures, maintaining clarity in the API contract.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Removed redundant test for parameter order in `mappings$get`, as this is now covered by a new comprehensive test in `test-roxygen-param-order.R`. - Introduced `test-roxygen-param-order.R` to ensure all documented R6 methods maintain parameter order consistency with their signatures, enhancing overall test coverage and clarity.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
- Added `pkgload` to the workflow's extra packages for improved testing capabilities. - Introduced a new step in the workflow to check the order of documented parameters against method signatures using `testthat`. - Updated the test for parameter order consistency to handle cases where the package source is not available, ensuring robust testing in different environments.
There was a problem hiding this comment.
All reported issues were addressed across 2 files (changes from recent commits).
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Summary by cubic
Adds pagination to concept mappings, adds
client$mappings$get_all()to return the full set, and introducesrelationship_idsandvocab_release. Keepsget()output shape and 1.8.1 positional arguments backward compatible; docs, examples, tests, and CI guard updated.New Features
client$mappings$get()is paginated withpage/page_size(default 100; max 200); pagination metadata inattr(result, "pagination").client$mappings$get_all()walks all pages; supportspage_size,max_pages,progress,vocab_release, andrelationship_ids.relationship_idsonget()/get_all()to select relationships (e.g.,c("Maps to", "Maps to value")).Bug Fixes
get()return shape after API pagination; pagination moved to an attribute.include_invalidis tri-state: defaultNULLuses server default; passFALSEto exclude.page_size200; maintain 1.8.1 positional order forget().@paramorder matches R6 method signatures; CI runs this guard on PRs and addsany::pkgloadto the workflow.Written for commit e88b6cd. Summary will update on new commits.