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.

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 IndexError for custom queryset managers #1913

Merged

Conversation

meshy
Copy link
Contributor

@meshy meshy commented Jan 22, 2024

Sometimes Mypy crashes on this line with an IndexError.

This change builds on the change in #1786 by making further use of the safer variable typed_var.

typed_var will equal manager_instance.args if manager_instance.args is populated, but if it isn't it will have another sensible value we can use instead.

Because that value should be populated, this prevents the crash.

Related issues

Original issue: #1785
Previous fix: #1786

Sometimes Mypy crashes on this line with an IndexError.

This change builds on the change in
typeddjango#1786 by making further
use of the safer variable `typed_var`.

`typed_var` will equal `manager_instance.args` if
`manager_instance.args` is populated, but if it isn't it will have
another sensible value we can use instead.

Because that value should be populated, this prevents the crash.
Copy link
Collaborator

@intgr intgr left a comment

Choose a reason for hiding this comment

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

Thanks. Sorry that this has sat around so long without review.

Would be useful to add a test for this edge case. Possibly based on the from_queryset_custom_manager_subclass test that was added to cover a similar case.

Copy link
Collaborator

@intgr intgr left a comment

Choose a reason for hiding this comment

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

Since the fix is straightforward this shouldn't cause any issues, I think it's OK to merge without a test.

@intgr intgr merged commit f7f2e61 into typeddjango:master Apr 30, 2024
40 checks passed
@intgr intgr added the mypy-plugin Issues specific to mypy_django_plugin label Apr 30, 2024
@meshy
Copy link
Contributor Author

meshy commented Apr 30, 2024

Apologies for not getting back to you on the tests. I've had this on my todo list, but wasn't able to prioritise it. Thank you for merging this!

@meshy meshy deleted the fix-custom-queryset-manager-indexerror branch April 30, 2024 13:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mypy-plugin Issues specific to mypy_django_plugin
Development

Successfully merging this pull request may close these issues.

2 participants