-
Notifications
You must be signed in to change notification settings - Fork 105
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
fix!(QueryList): Generic fixes #515
Merged
Merged
Conversation
This file contains 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
809130e
to
045445a
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #515 +/- ##
==========================================
+ Coverage 88.67% 89.06% +0.39%
==========================================
Files 35 36 +1
Lines 3522 3557 +35
Branches 488 502 +14
==========================================
+ Hits 3123 3168 +45
+ Misses 288 268 -20
- Partials 111 121 +10 ☔ View full report in Codecov by Sentry. |
src/libtmux/_internal/query_list.py:307: error: List comprehension has incompatible type List[tuple[Any, T]]; expected List[T] [misc] src/libtmux/_internal/query_list.py:376: error: Incompatible types in assignment (expression has type "Callable[[str | list[Any]], bool]", variable has type "Callable[[T], bool]") [assignment]
7cfeba1
to
cca33ec
Compare
cca33ec
to
a1d8d75
Compare
tests/test_dataclasses.py:166: error: Incompatible types in assignment (expression has type "Window | None", variable has type "Window") [assignment] tests/test_dataclasses.py:171: error: Incompatible types in assignment (expression has type "Window | None", variable has type "Window") [assignment]
72f944e
to
cb9f465
Compare
cb9f465
to
da78f52
Compare
da78f52
to
76bcfc8
Compare
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.
ChatGPT helped!
Part 1
Prompt
Assume this:
What would it take to make
QueryList
behave as a generic list type? Right now (as-is)QueryList[CustomObjectType]
is not a valid annotation according to mypy.Response:
Response
To make
QueryList
behave as a generic list type and ensure it's recognized as such by static type checkers like mypy, you need to ensure that it correctly inherits from a generic type and uses type annotations properly throughout its definition. The modifications needed involve leveraging the typing system to annotateQueryList
as a generic collection properly.Here are the steps to make
QueryList
a properly typed, generic list in Python:Ensure Proper Inheritance: The
QueryList
should inherit fromtyping.Generic
andcollections.abc.Sequence
(or another appropriate base class) to get list-like behavior while also being generic. You've already importedGeneric
and are usingTypeVar
, which is great. However, yourQueryList
definition should explicitly indicate it's generic over some typeT
.Use the TypeVar Correctly: You declared a
TypeVar
namedT
but didn't use it effectively withinQueryList
to ensure it's recognized as a generic class by type checkers. You should useT
as a placeholder for the type of items contained in theQueryList
.Annotate
data
Attribute: Thedata
attribute inQueryList
should be annotated to indicate it holds a sequence of typeT
elements. However, this attribute seems redundant ifQueryList
itself behaves like a list; you might be intending to useself
to store the data directly.Correct Method Annotations: Ensure all methods that operate on elements of the list use the
T
type variable appropriately. This includes methods that add, remove, retrieve, or operate on elements in any way, ensuring they are compatible with the generic typeT
.Update
__init__
Method: YourQueryList
doesn't explicitly define an__init__
method to initialize the list. It's important to properly initialize the internal list structure that will hold the data.Below is an updated example that incorporates these suggestions:
This example assumes
QueryList
directly inherits list-like behavior fromList[T]
and adds typing information to make it a generic class capable of holding items of any specified type. It demonstrates how to initialize it with items of typeT
and includes a placeholder for afilter
method that should be implemented according to your specific needs.Remember, when you're using
QueryList[CustomObjectType]
, theCustomObjectType
needs to be a class or type alias that mypy can recognize. Also, make sure your runtime logic correctly handles the types you expectQueryList
to contain, as Python's type annotations are not enforced at runtime.Part 2
Prompt
Consider the following:
What do I need to do to solve these mypy warnings:
Response:
Response
To resolve the mypy warnings you're encountering, you need to address type compatibility and the correct use of generics in your
QueryList
class. Let's go through each error and discuss the solutions:Error 1: List comprehension has incompatible type List[tuple[Any, T]]; expected List[T]
This error occurs in the
items
method. The method signature implies it returns a list of typeT
, but the actual implementation returns a list of tuples. To fix this, you should adjust either the method's return type to accurately reflect the returned data or change the method's functionality to match the expected return type. Since the method's purpose seems to be returning items along with their primary keys, adjusting the return type annotation is the appropriate solution.Solution:
Error 2: Incompatible types in assignment
This error indicates a mismatch in the expected callable type in your
filter
method. The_filter
variable is expected to be a callable that takes an item of typeT
and returns a boolean. However, the assignment toval_match
doesn't match this signature becauseval_match
is defined to take either a string or a list of any type. This discrepancy arises due to the flexible nature ofmatcher
, which can be a list, affecting the expected type of objects passed to_filter
.Solution:
To resolve this, you should ensure that the lambda or function assigned to
_filter
always matches the expected signature,(T) -> bool
. Ifmatcher
can be of different types, handle the type checking inside the callable, ensuring it always accepts an argument of typeT
and returns a boolean.In this solution,
val_match
assumes thatobj
can be compared directly to the items inmatcher
or tomatcher
itself. You might need to adjust this logic depending on the typesT
can represent, especially ifT
is not guaranteed to be comparable with the types inmatcher
.General Note on Type Checking and Generics
When working with generics and type checking in Python, especially with complex class behaviors like
QueryList
, it's crucial to ensure that all type annotations accurately reflect the intended use and behaviors of your class methods. This includes being precise about the types of arguments, return values, and the handling of generic type variables throughout the class. Remember that mypy's type checking is based on the static analysis of these annotations, so discrepancies between the annotations and the actual code logic will lead to warnings or errors.