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

Unpin mypy #35667

Open
kdmccormick opened this issue Oct 17, 2024 · 4 comments · May be fixed by #36072
Open

Unpin mypy #35667

kdmccormick opened this issue Oct 17, 2024 · 4 comments · May be fixed by #36072
Assignees
@kdmccormick

Comments

@kdmccormick
Copy link
Member

kdmccormick commented Oct 17, 2024 •
edited
Loading

mypy was recently upgraded to 1.12.0, and CI ran on that upgrade PR without issue.

Once that PR was merged in with the changes from this collections PR, though, mypy on master started failing with:

openedx/core/djangoapps/content_libraries/api.py:737: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 1.12.0
openedx/core/djangoapps/content_libraries/api.py:737: : note: please use --show-traceback to print a traceback when reporting a bug

The "offending" line does not have anything obviously wrong with it:

edx-platform/openedx/core/djangoapps/content_libraries/api.py

Line 737 in ecbedbc

associated_collections = None

--show-traceback adds more details:

app@4995b0c986b2:~/edx-platform$ mypy --show-traceback
openedx/core/djangoapps/content_libraries/api.py:737: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.12.0
Traceback (most recent call last):
  File "mypy/checkexpr.py", line 5847, in accept
  File "mypy/nodes.py", line 1969, in accept
  File "mypy/checkexpr.py", line 480, in visit_call_expr
  File "mypy/checkexpr.py", line 614, in visit_call_expr_inner
  File "mypy/checkexpr.py", line 1471, in check_call_expr_with_callee_type
  File "mypy/checkexpr.py", line 1565, in check_call
  File "mypy/checkexpr.py", line 1811, in check_callable_call
  File "mypy/checkexpr.py", line 1262, in apply_function_plugin
  File "/openedx/venv/lib/python3.11/site-packages/mypy_django_plugin/transformers/querysets.py", line 305, in extract_proper_type_queryset_values
    row_type = helpers.make_typeddict(ctx.api, column_types, set(column_types.keys()))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/openedx/venv/lib/python3.11/site-packages/mypy_django_plugin/lib/helpers.py", line 307, in make_typeddict
    typed_dict_type = TypedDictType(fields, required_keys=required_keys, fallback=object_type)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: __init__() missing required argument 'readonly_keys' (pos 3)
openedx/core/djangoapps/content_libraries/api.py:737: : note: use --pdb to drop into pdb

We are pinning mypy back to 1.11.2 here: #35666
@bmtcril
Copy link
Contributor

bmtcril commented Oct 17, 2024

This actually seems to be an issue with us pinning django-stubs here: #35275 I think we can move forward with this unpinning, but am not certain enough to do it today.

@bmtcril bmtcril mentioned this issue Oct 17, 2024
Merged
@kdmccormick kdmccormick self-assigned this Dec 2, 2024
@kdmccormick
Copy link
Member Author

Trying this out now that django-stubs and djangorestframework-stubs are unpinned...

kdmccormick added a commit to kdmccormick/edx-platform that referenced this issue Jan 2, 2025
@kdmccormick
build: Unpin mypy and upgrade it to latest
669c6a9 
This is possible now that django-stubs and djangorestframework-stubs
are unpinned:
openedx@a5b773c

which became possible once we upgraded to django 4.2.

Closes: openedx#35667
@kdmccormick kdmccormick linked a pull request Jan 2, 2025 that will close this issue
Draft
@kdmccormick
Copy link
Member Author

Let's see if this passes CI: #36072

@kdmccormick
Copy link
Member Author

The above PR hit an internal error with the latest mypy and django-stubs==4.2.1:

app@6d404f8878ce:~/edx-platform$ mypy openedx/core/djangoapps/content_libraries/api.py --show-traceback
openedx/core/djangoapps/content_libraries/api.py:744: error: INTERNAL ERROR -- Please try using mypy master on GitHub:
https://mypy.readthedocs.io/en/stable/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 1.14.1
Traceback (most recent call last):
  File "mypy/checkexpr.py", line 5890, in accept
  File "mypy/nodes.py", line 1984, in accept
  File "mypy/checkexpr.py", line 484, in visit_call_expr
  File "mypy/checkexpr.py", line 618, in visit_call_expr_inner
  File "mypy/checkexpr.py", line 1475, in check_call_expr_with_callee_type
  File "mypy/checkexpr.py", line 1571, in check_call
  File "mypy/checkexpr.py", line 1817, in check_callable_call
  File "mypy/checkexpr.py", line 1266, in apply_function_plugin
  File "/openedx/venv/lib/python3.11/site-packages/mypy_django_plugin/transformers/querysets.py", line 311, in extract_proper_type_queryset_values
    row_type = helpers.make_typeddict(ctx.api, column_types, set(column_types.keys()))
               ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/openedx/venv/lib/python3.11/site-packages/mypy_django_plugin/lib/helpers.py", line 355, in make_typeddict
    typed_dict_type = TypedDictType(fields, required_keys=required_keys, fallback=fallback_type)
                      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: __init__() missing required argument 'readonly_keys' (pos 3)

The issue was a known bug which was fixed in django-stubs v5.1.1, per python/mypy#17958.

So, we'll need to bump django-stubs further forward in order to upgrade mypy.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@kdmccormick kdmccormick

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

build: Unpin mypy and upgrade it to latest kdmccormick/edx-platform
2 participants
@kdmccormick @bmtcril