Fixes to choices support in plugin #2646
Merged
Conversation
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
)" This reverts commit 7b4145d.
Without these we cannot import from other modules properly which is useful for testing the behaviour of resolving types in the plugin.
Add some comments to explain what is being tested in each block of `assert_type()` calls.
The resolution of the attribute type was too naive and didn't handle where a module is imported and the choices type is accessed as an attribute from that module. Eliminates 11,087 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55. Co-authored-by: Stephen Moore <[email protected]>
The resolution of the attribute type was too naive and didn't handle where a choices type is aliased and then used via that alias. Eliminates 53 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55. Co-authored-by: Stephen Moore <[email protected]>
The resolution of the attribute type was too naive and didn't handle where a choices type is aliased by type and then used via that alias. Eliminates 37 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55. Co-authored-by: Stephen Moore <[email protected]>
The resolution of the attribute type was too naive and didn't handle where a choices type is bound in a type variable. Eliminates 10 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55. Co-authored-by: Stephen Moore <[email protected]>
The resolution of the attribute type was too naive and didn't handle where a choices type is decomposed to a union of enum literals when a comparison to a member is made in a branch. Eliminates 33 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55. Co-authored-by: Stephen Moore <[email protected]>
The resolution of the attribute type was too naive and didn't handle where a choices type overrides a property, e.g. label, and makes use of `super()`. Eliminates 1 error in a large repository that was raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55.
The resolution of the attribute type was too naive and didn't handle where a choices type is decomposed to a single enum literal when a comparison to a member is made in a branch. Eliminates 1 error in a large repository that was raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55.
Ahead of a future commit that alters handling of labels, perform this simplification.
Because of the change to use `django.utils.functional._StrOrPromise` for labels to improve accuracy, it can be a little painful - especially in a very large codebase where translations / lazy strings are not used. This change keeps the stubs unchanged for correctness, e.g. in pyright for where there is no alternative, but tweaks the mypy plugin to handle introspecting the members to determine whether there are no lazy strings such that the types can be narrowed. Eliminates 1,166 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55.
This case is a little complex as we have a mix of choices types. In theory we could handle this, but it would require more rework to handle multiple types. For now, just make no changes and return the original type. Eliminates 11 errors in a large repository that were raised when upgrading from django-stubs v5.1.3 to v5.2.0 which included the changes in a3f2a55.
ngnpope
force-pushed
the
tweaks-to-choices-plugin
branch
from
0102d08 to
ddd6a2b
Compare
sobolevn
reviewed
Apr 29, 2025
There was a problem hiding this comment.
Wow! That's a lot of changes :)
I would like to see some second opinion from @adamchainz or @flaeppe
Thanks!
delfick
reviewed
Apr 29, 2025
sobolevn
approved these changes
Apr 30, 2025
ngnpope
added a commit
to ngnpope/django-stubs
that referenced
this pull request
May 9, 2025
Currently mypy v1.13 is the minimum supported version, but the fix made to the choices plugin code in typeddjango#2646 made use of `TypeInfo.enum_members` which was introduced in v1.14. This commit adds a backport of the version introduced in v1.14 which can be removed when `django-stubs` bumps its minimum version.
sobolevn
pushed a commit
that referenced
this pull request
May 9, 2025
Currently mypy v1.13 is the minimum supported version, but the fix made to the choices plugin code in #2646 made use of `TypeInfo.enum_members` which was introduced in v1.14. This commit adds a backport of the version introduced in v1.14 which can be removed when `django-stubs` bumps its minimum version.
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.
I have made things!
So I was far too naive in #2582 🤦🏻
I've run this on a very large repository and this now fixes all of the
Unable to resolve type of ... propertyerrors I encountered. I also made a change to narrow the label type from_StrOrPromisetostrwhere all members labels and__empty__are not using lazy strings. Although the changes to use_StrOrPromiseare correct according to the implementation, it can be annoying to fix up in large projects that are only using plain strings for labels. This change to narrow the type will vastly reduce the number of errors raised and make upgrading less painful.Related issues
Unable to resolve type of choices propertyfor.valueon Choices #2643.valueof a Choices variable #2644