Skip to content
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.

Sign up for GitHub

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

Jump to bottom

use field annotations for values_list types #2248

Merged
merged 2 commits into from
Jul 4, 2024
Merged

use field annotations for values_list types #2248

merged 2 commits into from
Jul 4, 2024

Conversation

asottile
Copy link
Contributor

@asottile asottile commented Jul 3, 2024

test originally failing with:

 N: Revealed type is "builtins.list[Tuple[builtins.int, builtins.str]]

I reorganized the similar setter type determination as well as they had symmetry

@sobolevn sobolevn requested a review from flaeppe July 3, 2024 20:42
flaeppe
flaeppe approved these changes Jul 4, 2024
View reviewed changes
Copy link
Member

@flaeppe flaeppe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work

@flaeppe flaeppe merged commit 085b91b into typeddjango:master Jul 4, 2024
36 checks passed
@asottile asottile deleted the values-list-set-type branch July 4, 2024 20:16
asottile-sentry pushed a commit to getsentry/sentry-forked-django-stubs that referenced this pull request Jul 22, 2024
@asottile @pre-commit-ci @asottile-sentry
use field annotations for values_list types (typeddjango#2248)
fd35b99 
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@asottile-sentry asottile-sentry mentioned this pull request Jul 22, 2024
Merged
asottile-sentry added a commit to getsentry/sentry-forked-django-stubs that referenced this pull request Jul 22, 2024
@asottile-sentry @asottile @pre-commit-ci
use field annotations for values_list types (typeddjango#2248) (#20)
2f287e6 
Co-authored-by: Anthony Sottile <[email protected]>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
@asottile-sentry asottile-sentry mentioned this pull request Jul 29, 2024
Merged
asottile-sentry added a commit to getsentry/sentry that referenced this pull request Jul 29, 2024
@asottile-sentry
ref: get proper base manager typing (#72221)
89697c3 
an absolute ton of work went into enabling this -- but after this PR
model types / create / update / etc. should mostly be typechecked
whereas they were entirely unchecked (everything `Any`) prior to this.

most of the core problem was that mypy and django-stubs did not
understand our custom `BaseManager` and `BaseQuerySet` since there's a
significant amount of django stuff that goes into making those classes
"real"

fix that involved these issues:

- python/mypy#17402 (worked around)
- typeddjango/django-stubs#2228 (the workaround)

with that fixed, it exposed a handful issues in django-stubs itself:

- typeddjango/django-stubs#2240
- typeddjango/django-stubs#2241
- typeddjango/django-stubs#2244
- typeddjango/django-stubs#2248
- typeddjango/django-stubs#2276
- typeddjango/django-stubs#2281

fixing all of those together exposed around 1k issues in sentry code
itself which was fixed over a number of PRs:
- #75186
- #75108
- #75149
- #75162
- #75150
- #75154
- #75158
- #75148
- #75157
- #75156
- #75152
- #75153
- #75151
- #75113
- #75112
- #75111
- #75110
- #75109
- #75013
- #74641
- #74640
- #73783
- #73787
- #73788
- #73793
- #73791
- #73786
- #73761
- #73742
- #73744
- #73602
- #73596
- #73595
- #72892
- #73589
- #73587
- #73581
- #73580
- #73213
- #73207
- #73206
- #73205
- #73202
- #73198
- #73121
- #73109
- #73073
- #73061
- getsentry/getsentry#14370
- #72965
- #72963
- #72967
- #72877 (eventually was able to remove this when upgrading to mypy
1.11)
- #72948
- #72799
- #72898
- #72899
- #72896
- #72895
- #72903
- #72894
- #72897
- #72893
- #72889
- #72887
- #72888
- #72811
- #72872
- #72813
- #72815
- #72812
- #72808
- #72802
- #72807
- #72809
- #72810
- #72814
- #72816
- #72818
- #72806
- #72801
- #72797
- #72800
- #72798
- #72771
- #72718
- #72719
- #72710
- #72709
- #72706
- #72693
- #72641
- #72591
- #72635


mypy 1.11 also included some important improvements with typechecking
`for model_cls in (M1, M2, M2): ...` which also exposed some problems in
our code. unfortunately upgrading mypy involved fixing a lot of things
as well:
- #75020
- #74982
- #74976
- #74974
- #74972
- #74967
- #74954
- getsentry/getsentry#14739
- getsentry/getsentry#14734
- #74959
- #74958
- #74956
- #74953
- #74955
- #74952
- #74895
- #74892
- #74897
- #74896
- #74893
- #74880
- #74900
- #74902
- #74903
- #74904
- #74899
- #74894
- #74882
- #74881
- #74871
- #74870
- #74855
- #74856
- #74853
- #74857
- #74858
- #74807
- #74805
- #74803
- #74806
- #74798
- #74809
- #74801
- #74804
- #74800
- #74799
- #74725
- #74682
- #74677
- #74680
- #74683
- #74681


needless to say this is probably the largest stacked PR I've ever done
-- and I'm pretty happy with how I was able to split this up into
digestable chunks

<!-- Describe your PR here. -->
roaga pushed a commit to getsentry/sentry that referenced this pull request Jul 31, 2024
@asottile-sentry @roaga
ref: get proper base manager typing (#72221)
2553119 
an absolute ton of work went into enabling this -- but after this PR
model types / create / update / etc. should mostly be typechecked
whereas they were entirely unchecked (everything `Any`) prior to this.

most of the core problem was that mypy and django-stubs did not
understand our custom `BaseManager` and `BaseQuerySet` since there's a
significant amount of django stuff that goes into making those classes
"real"

fix that involved these issues:

- python/mypy#17402 (worked around)
- typeddjango/django-stubs#2228 (the workaround)

with that fixed, it exposed a handful issues in django-stubs itself:

- typeddjango/django-stubs#2240
- typeddjango/django-stubs#2241
- typeddjango/django-stubs#2244
- typeddjango/django-stubs#2248
- typeddjango/django-stubs#2276
- typeddjango/django-stubs#2281

fixing all of those together exposed around 1k issues in sentry code
itself which was fixed over a number of PRs:
- #75186
- #75108
- #75149
- #75162
- #75150
- #75154
- #75158
- #75148
- #75157
- #75156
- #75152
- #75153
- #75151
- #75113
- #75112
- #75111
- #75110
- #75109
- #75013
- #74641
- #74640
- #73783
- #73787
- #73788
- #73793
- #73791
- #73786
- #73761
- #73742
- #73744
- #73602
- #73596
- #73595
- #72892
- #73589
- #73587
- #73581
- #73580
- #73213
- #73207
- #73206
- #73205
- #73202
- #73198
- #73121
- #73109
- #73073
- #73061
- getsentry/getsentry#14370
- #72965
- #72963
- #72967
- #72877 (eventually was able to remove this when upgrading to mypy
1.11)
- #72948
- #72799
- #72898
- #72899
- #72896
- #72895
- #72903
- #72894
- #72897
- #72893
- #72889
- #72887
- #72888
- #72811
- #72872
- #72813
- #72815
- #72812
- #72808
- #72802
- #72807
- #72809
- #72810
- #72814
- #72816
- #72818
- #72806
- #72801
- #72797
- #72800
- #72798
- #72771
- #72718
- #72719
- #72710
- #72709
- #72706
- #72693
- #72641
- #72591
- #72635


mypy 1.11 also included some important improvements with typechecking
`for model_cls in (M1, M2, M2): ...` which also exposed some problems in
our code. unfortunately upgrading mypy involved fixing a lot of things
as well:
- #75020
- #74982
- #74976
- #74974
- #74972
- #74967
- #74954
- getsentry/getsentry#14739
- getsentry/getsentry#14734
- #74959
- #74958
- #74956
- #74953
- #74955
- #74952
- #74895
- #74892
- #74897
- #74896
- #74893
- #74880
- #74900
- #74902
- #74903
- #74904
- #74899
- #74894
- #74882
- #74881
- #74871
- #74870
- #74855
- #74856
- #74853
- #74857
- #74858
- #74807
- #74805
- #74803
- #74806
- #74798
- #74809
- #74801
- #74804
- #74800
- #74799
- #74725
- #74682
- #74677
- #74680
- #74683
- #74681


needless to say this is probably the largest stacked PR I've ever done
-- and I'm pretty happy with how I was able to split this up into
digestable chunks

<!-- Describe your PR here. -->
@flaeppe flaeppe mentioned this pull request Aug 10, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@flaeppe flaeppe flaeppe approved these changes

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@asottile @flaeppe