-
Notifications
You must be signed in to change notification settings - Fork 1.8k
feat: add support for deleting state attributes, implements the '__delitem__' method on the 'State class' #2678
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
base: main
Are you sure you want to change the base?
Conversation
…litem__' method on the 'State class'
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary of Changes
Hello @Amar3x3, 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 introduces the capability to delete attributes from the State
class, resolving a long-standing issue where attempts to remove state entries would result in an AttributeError
. By implementing the __delitem__
method, users can now seamlessly remove attributes using standard Python syntax, enhancing the flexibility and manageability of session state. The changes ensure that deletions are consistently applied across the state's internal data structures and are thoroughly validated with new unit tests.
Highlights
- Enabling Attribute Deletion: The core change is the addition of the
__delitem__
magic method to theState
class. This method enables the use of Python's built-indel
keyword to remove attributes from the session state, addressing a previously reportedAttributeError
and feature request #2564. - Ensuring State Consistency: The implementation of
__delitem__
ensures data consistency by checking for and removing the specified key from both the_value
(base state) and_delta
(pending changes) dictionaries within theState
object. This prevents orphaned or inconsistent state entries. - Comprehensive Test Coverage: New unit tests have been added to thoroughly validate the
__delitem__
functionality, including scenarios for successful deletion from both internal dictionaries and properKeyError
handling for non-existent keys. An additional test file was created to specifically reproduce the originalAttributeError
.
Using Gemini Code Assist
The 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 in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command>
or @gemini-code-assist <command>
. Below is a summary of the supported commands.
Feature | Command | Description |
---|---|---|
Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
Help | /gemini help |
Displays a list of available commands. |
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 .gemini/
folder in the base of the repository. Detailed instructions can be found here.
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.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
Response from ADK Triaging Agent Hello @Amar3x3, thank you for creating this PR! It looks like the CLA check has failed. Please make sure to sign the Contributor License Agreement (CLA) for your contribution to be accepted. In addition, could you please add a Thanks! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code Review
This pull request successfully implements the __delitem__
method on the State
class, allowing for attribute deletion. The implementation is correct and is accompanied by a good set of unit tests in test_state.py
.
My review includes a few suggestions:
- A simplification of the
__delitem__
implementation for better readability and conciseness. - A recommendation to remove the temporary test script
test_delitem_issue.py
as its purpose is now covered by the formal unit tests. - An enhancement to the new unit tests to cover an additional edge case where a key exists in both internal dictionaries.
Overall, this is a solid contribution that addresses the feature request.
@DeanChensj delete state attributes is added, with unit test cases, please look into it and let me know if any improvements and changes required |
Hi! I have similar PR for the same functionallity. Your PR has some problems because deleting state key is not enought. You can check comments of my PR I've faced the same problem. If you want you can help me write tests! :) |
Attribute Deletion: Implements the delitem method, allowing users to remove an attribute from the session state using the del callback_context.state['attribute-id'] syntax. This resolves the AttributeError: delitem and directly addresses feature request #2564 .
Implementation Details
delitem(key): The method checks for the key's existence and removes it from both the base _value and the pending _delta dictionaries to ensure the state is consistent. It raises a KeyError if the key is not found.