Skip to content

v1.9.0 prep - #10

Merged
alex-omophub merged 12 commits into
mainfrom
develop
Aug 12, 2026
Merged

v1.9.0 prep#10
alex-omophub merged 12 commits into
mainfrom
develop

Conversation

@alex-omophub

@alex-omophub alex-omophub commented Aug 11, 2026

Copy link
Copy Markdown
Member

Summary by cubic

Adds pagination to concept mappings, adds client$mappings$get_all() to return the full set, and introduces relationship_ids and vocab_release. Keeps get() output shape and 1.8.1 positional arguments backward compatible; docs, examples, tests, and CI guard updated.

  • New Features

    • client$mappings$get() is paginated with page/page_size (default 100; max 200); pagination metadata in attr(result, "pagination").
    • client$mappings$get_all() walks all pages; supports page_size, max_pages, progress, vocab_release, and relationship_ids.
    • relationship_ids on get()/get_all() to select relationships (e.g., c("Maps to", "Maps to value")).
  • Bug Fixes

    • Preserve get() return shape after API pagination; pagination moved to an attribute.
    • include_invalid is tri-state: default NULL uses server default; pass FALSE to exclude.
    • Validate pagination inputs; enforce max page_size 200; maintain 1.8.1 positional order for get().
    • Package-wide test ensures documented @param order matches R6 method signatures; CI runs this guard on PRs and adds any::pkgload to the workflow.

Written for commit e88b6cd. Summary will update on new commits.

Review in cubic

alex-omophub added 6 commits August 11, 2026 22:01
- 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread R/mappings.R Outdated
Comment thread R/mappings.R
…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

codecov Bot commented Aug 12, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All reported issues were addressed across 3 files (changes from recent commits).

Reply with feedback, questions, or to request a fix.

Re-trigger cubic

Comment thread R/mappings.R
- 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/testthat/test-mappings.R Outdated
- 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/testthat/test-roxygen-param-order.R Outdated
alex-omophub added 2 commits August 12, 2026 10:18
- 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.

@cubic-dev-ai cubic-dev-ai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Comment thread tests/testthat/test-roxygen-param-order.R Outdated
@alex-omophub
alex-omophub merged commit f37e220 into main Aug 12, 2026
14 checks passed
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

Successfully merging this pull request may close these issues.

1 participant