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

type[Model] no longer allows access to .objects in django-stubs 4.2.4 #1684

Closed
johnthagen opened this issue Sep 5, 2023 · 11 comments
Closed
Labels
bug Something isn't working

Comments

@johnthagen
Copy link

johnthagen commented Sep 5, 2023

Bug report

What's wrong

Prior to django-stubs 4.2.4 / mypy 1.5.1, the you could type hint a variable/class attribute as type[Model] and access .objects on it:

from django.contrib.auth.models import User
from django.db.models import Model

user_type: type[Model] = User

users = user_type.objects.all()

Note that is the real usage, the variable is a class variable that is set by derived classes.

But as of the latest django-stubs and mypy, this generates a type check error:

error: "type[Model]" has no attribute "objects"  [attr-defined]

How is that should be

No type check errors.

System information

  • OS: macOS
  • python version: 3.10.11
  • django version: 4.2.5
  • mypy version: 1.5.1
  • django-stubs version: 4.2.4
  • django-stubs-ext version: 4.2.2

Mypy configuration:

[tool.mypy]
ignore_missing_imports = true
strict = true

disallow_subclassing_any = false
disallow_untyped_decorators = false
warn_return_any = false

plugins = [
    "pydantic.mypy",
    "mypy_django_plugin.main",
]
@johnthagen johnthagen added the bug Something isn't working label Sep 5, 2023
@flaeppe
Copy link
Member

flaeppe commented Sep 5, 2023

Yes, models.Model is no longer assumed to have the objects manager. It's not guaranteed to exist for a models.Model. You should use the _default_manager attribute instead: https://docs.djangoproject.com/en/4.2/topics/db/managers/#django.db.models.Model._default_manager

If you’re writing some code that must handle an unknown model, for example, in a third-party app that implements a generic view, use this manager (or _base_manager) rather than assuming the model has an objects manager.

from django.contrib.auth.models import User
from django.db.models import Model

user_type: type[Model] = User

users = user_type._default_manager.all()

@flaeppe flaeppe closed this as completed Sep 5, 2023
@johnthagen
Copy link
Author

Thanks for the tip @flaeppe!

@sobolevn
Copy link
Member

sobolevn commented Sep 5, 2023

I think that we might need to put this in the docs!

@djbrown
Copy link
Contributor

djbrown commented Sep 10, 2023

But when I switch to model._default_manager I get the following pylint error:
W0212: Access to a protected member _default_manager of a client class (protected-access)
So either I stay with model.objects and add a mypy ignore line comment,
or go with model._default_manager and add a pylint ignore line comment.. 😞

djbrown added a commit to djbrown/hbscorez that referenced this issue Sep 10, 2023
@johnthagen
Copy link
Author

johnthagen commented Sep 11, 2023

Yeah, PyCharm also warns about accessing a private member (in this case, it's "only" a yellow squiggle, and doesn't show up in CI). Ruff doesn't warn about this, which I suppose is nice, depending on your point of view.

Is this fundamentally a Django issue that should be raised with them? It seems like in general it shouldn't be prescribed that end users use private methods and attributes?

@sobolevn
Copy link
Member

We have our own Django expert to ask :)
CC @adamchainz

@meshy
Copy link
Contributor

meshy commented Feb 2, 2024

It's not guaranteed to exist for a models.Model. You should use the _default_manager attribute instead

I don't think that this follows. The quoted text only says that we should use _default_manager "if you’re writing some code that must handle an unknown model, for example, in a third-party app that implements a generic view." (Emphasis mine.)

I sympathise that it's a very frustrating choice on Django's part to only sometimes create the objects attribute dynamically, but if we follow this advice, many typed Django projects will end up using _default_manager instead of objects, even when that's not required. I think that's a bad thing.

It seems like in general it shouldn't be prescribed that end users use private methods and attributes?

I wholeheartedly agree.

Is there some other possible work-around? Should all models explicitly define objects, or find some mechanism for making it easier to detect when the attribute does not exist?

It's not guaranteed to exist for a models.Model.

Is this fundamentally a Django issue that should be raised with them?

I think that it might well be. Perhaps the solution here is that we should try to get this behaviour changed in Django.

@meshy
Copy link
Contributor

meshy commented Feb 6, 2024

A quick follow-up to my comment above:

Adding a objects: models.Manager["MyModel"] type annotation to models without an explicit manager declaration is enough to get Mypy picking up on the .objects again.

We're using that instead of replacing .objects with ._default_manager in our codebase.

@djbrown
Copy link
Contributor

djbrown commented Mar 25, 2024

Adding a objects: models.Manager["MyModel"] type annotation to models without an explicit manager declaration is enough to get Mypy picking up on the .objects again.

Nice workaround 👍🏾 doesn't work for generic code though 🙁

@delfick
Copy link
Contributor

delfick commented Mar 27, 2024

Adding a objects: models.Manager["MyModel"] type annotation to models without an explicit manager declaration is enough to get Mypy picking up on the .objects again.

I'm working on the same codebase, It turns out that solution causes a bunch of other problems and we weren't able to proceed with it. And ultimately the problem is working with abstract models instead of concrete models is a headache. Made worse when what concrete models are available at runtime depends on django settings in a way that means we can't explicitly refer to them in many places

@delfick
Copy link
Contributor

delfick commented Dec 3, 2024

I've made this discussion to talk about how we dealt with this problem in our codebase, #2452

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

No branches or pull requests

6 participants