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

Break what apparently is a cycle involving custom User model and QuerySet.as_manager() #2377

Merged

Conversation

sterliakov
Copy link
Contributor

Break a cycle resulting in [django-manager-missing]

I'm not sure if my treatment is correct, perhaps I'm just treating symptoms of some deeper problem. However, this at least makes a test case extracted from my real system pass, and removes a dozen of django-manager-missing errors in my code (actually all of them!).

See the contributed test case for details - I don't really see how to summarize it any better.

There seems to be some cycle during semanal, where base_as_manager.type remains None during all iterations (yes, I tried deferring first). This seems to only affect the base QuerySet and require custom AUTH_USER_MODEL.

Prior to the fix, the test failed with the following:

main:2: note: Revealed type is "payments.models.UnknownManager[payments.models.Payout]" (diff)
payments/models:10: error: Couldn't resolve related manager 'payouts' for relation 'payments.models.Payout.triggered_by'.  [django-manager-missing] (diff)
payments/models:24: error: Could not resolve manager type for "payments.models.Payout.objects"  [django-manager-missing] (diff)

@sterliakov
Copy link
Contributor Author

sterliakov commented Sep 20, 2024

@flaeppe honestly, I don't really like this one - feels like a hack. If you have some time, could you have a look into what might be going on here? I already spent ~2 days here, but can't figure out why QuerySet.as_manager remains unresolved after multiple semanal passes.

@flaeppe
Copy link
Member

flaeppe commented Sep 20, 2024

I'm not sure you can expect something to get fully typed during semantic analysis? I think you need the semantic analysis of types, type analysis or whatever, and then use a hook for when mypy has decided to enter that phase.

I can't say I'm totally sure about this stuff, but perhaps it could be of some help or give some pointers or something

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.

I think this looks fine. It adds special treatment to the QuerySet.as_manager method. Apparently because we can't expect it to have gotten types when the get_base_class_hook comes around.

We also add expectations on what its return value is, which is a bit of cheating, but it's not likely to change and a fair compromise.

@flaeppe flaeppe merged commit 0fb3bfd into typeddjango:master Sep 21, 2024
36 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants