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 type annotations for 3rd party models #2440

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

Niicck
Copy link
Contributor

@Niicck Niicck commented Nov 10, 2024

Fix type annotations for 3rd party models

The clean-up in PR #2263 that got released in v5.0.3 ended up breaking type annotations for models that are subclasses of models imported from 3rd party libraries.

I restored the logic that checks a model's fullname in the list of registered django apps. We still need to do this because Django models imported from untyped 3rd party libraries will have no metaclass_type, so helpers.is_model_type will always fail for them.

Related issues

@sobolevn sobolevn requested a review from flaeppe November 10, 2024 08:04
- django_extensions
- myapp
main: |
from myapp.models import A
Copy link
Member

Choose a reason for hiding this comment

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

please, add a reveal_type to A().name

- path: myapp/models.py
content: |
from django.db import models
from django_extensions.db.models import TimeStampedModel # type: ignore[import-untyped]
Copy link
Member

@flaeppe flaeppe Nov 10, 2024

Choose a reason for hiding this comment

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

I actually disagree with the changes here. The main issue in #2341 is that django-extensions is a package that doesn't export any type hints.

The ignore comment I'm highlighting here is saying precisely that. What is experienced in #2341 is how mypy behaves when there are no type hints.

What is attempted to be repaired here is only the first line of issues, there could be an arbitrary amount of other problems to solve after this one, coming from it being an untyped package.

There are multiple other approaches to resolve this, that integrates better with the type checker. e.g.

  • Add and export type hints in django-extensions
  • Create a stubs package(.pyi) just like django-stubs for django-extrnsions
  • Add stubs (.pyi-files) for django-extensions to your project, locally
  • Perhaps django-stubs would consider including stubs in a similar fashion to typeshed, that includes/maintains a bunch of 3rd party stubs?

Regardless, I think merging the changes here is a step in the wrong direction as it's outside of django-stubs domain

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.

Type Annotation Errors in Models Inheriting from 3rd Party Packages
3 participants