Make Suite.append() chainable (return self)#2317
Make Suite.append() chainable (return self)#2317kevinmessiaen merged 4 commits intoGiskard-AI:mainfrom
Conversation
Suite.append() 原本返回 None,无法链式调用如 suite.append(a).append(b)。 改为返回 self 并更新类型标注和 docstring。 Fixes Giskard-AI#2315
根据 Codex 审查反馈: - 返回类型从 "Suite[InputType, OutputType]" 改为 Self,与 Scenario 等同包 fluent API 保持一致 - 新增 test_suite_append_returns_self 和 test_suite_append_chaining 两个测试用例
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request enhances the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request makes the Suite.append() method chainable by having it return self. The change is well-implemented, follows the fluent API pattern seen elsewhere in the codebase, and is accompanied by new tests. My only suggestion is to enhance one of the new tests to be more specific about what it verifies, ensuring the scenarios are appended in the correct order.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
kevinmessiaen
left a comment
There was a problem hiding this comment.
Thanks for your contribution! Looking good
Description
Make
Suite.append()returnself(typed asSelf) so it supports fluent chaining likesuite.append(a).append(b). This aligns with the existing fluent API pattern used throughout the codebase (e.g.Scenario.interact().check()).Related Issue
Fixes #2315
Type of Change
Reproduction
Fix Scope
libs/giskard-checks/src/giskard/checks/scenarios/suite.py: Changedappend()return type fromNonetoSelfand addedreturn self. Updated docstring (numpydoc Returns section + examples).libs/giskard-checks/tests/core/test_suite.py: Addedtest_suite_append_returns_selfandtest_suite_append_chainingregression tests.Test Evidence
Two new tests added:
test_suite_append_returns_self— assertssuite.append(a) is suitetest_suite_append_chaining— chains.append(a).append(b), verifieslen(suite.scenarios) == 2andsuite.run()completes successfully.Known Limitations
None. The change is fully backward-compatible — existing code that ignores the return value of
append()is unaffected.Checklist
CODE_OF_CONDUCT.mddocument.CONTRIBUTING.mdguide.pdm.lockrunningpdm update-lock(only applicable whenpyproject.tomlhas been modified)