Open
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new generic Query type to improve type inference for query responses, replacing the previous use of Any. Key changes include:
- Adding a new covariant type variable ReturnType_co and Query protocol to declare query return types.
- Updating the query() method signature in the message bus to use the new Query[T] and generic return type.
- Adjusting documentation and tests to reflect the updated typing and usage of queries.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/banshee/bus.py | Introduces Query protocol and updates type annotations in query() |
| docs/usage/dispatching-requests.md | Updates documentation with examples for using the new Query type |
| src/banshee/init.py | Exports the new Query type |
| docs/conf.py | Updates Python intersphinx mapping |
| tests/unit/test_message_bus.py | Adjusts test annotations for query() return type |
| tests/unit/test_traceable_bus.py | Adjusts test annotations for query() return type |
Comment on lines
+81
to
+83
| class GetUserQuery(banshee.Query[User]): | ||
| user_id: int | ||
|
|
There was a problem hiding this comment.
The example snippet defines 'GetUserQuery' twice, which could be confusing. Consider removing or clarifying one of the definitions to avoid ambiguity.
Suggested change
| class GetUserQuery(banshee.Query[User]): | |
| user_id: int |
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.
Description
Adds a generic
Querytype that can be extended to define the return type of dispatching the query.Fixes #99
Motivation and Context
The current
Anyresponse has a type does not allow static analysis of the returned value.How Has This Been Tested?
Manually tested with mypy and vscode type inference (pyright) to check desired type is infered, defaulting to
Any.Screenshots:
Types of changes
Checklist: