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

Make mypy a soft dependency #1628

Closed
jorenham opened this issue Jul 14, 2023 · 19 comments · Fixed by #1782
Closed

Make mypy a soft dependency #1628

jorenham opened this issue Jul 14, 2023 · 19 comments · Fixed by #1782
Labels
enhancement New feature or request meta Meta-issues and discussion pyright Related to pyright type checker

Comments

@jorenham
Copy link
Contributor

Given that there are several popular alternatives to mypy (e.g. pyright and pytype), mypy should be an optional dependency, installable with e.g. django-stubs[mypy].

I haven't tested it myself yet, but if django-stubs doesn't work with these "alternative" typecheckers, then I'd suggest that it should explicitly mentioned that this is a mypy-only stubs package.

@jorenham jorenham added the bug Something isn't working label Jul 14, 2023
@j-osephlong
Copy link

+1 to this, many people (including me) primarily use the pyright implementation in Pylance, which as far as I can tell works fine with django-stubs.

@intgr intgr added enhancement New feature or request meta Meta-issues and discussion and removed bug Something isn't working labels Jul 17, 2023
@intgr
Copy link
Collaborator

intgr commented Jul 17, 2023

I agree, we should change this.

Other type checkers won't be able to use mypy_django_plugin for ORM and settings integration etc, but the .pyi type stubs should work fine.

Personally I don't see the need to add another explicit "extra" like django-stubs[mypy]. Projects that use mypy directly should have it as a direct dependency. And we already have the django-stubs[compatible-mypy] extra, which does depend on mypy.

Any thoughts @sobolevn @adamchainz or others?

@intgr intgr changed the title make mypy a soft dependency Make mypy a soft dependency Jul 17, 2023
@sobolevn
Copy link
Member

I think that most people have mypy and django-stubs in requirements anyway or django[compatible-mypy] if they want to have the mypy version we officially support.

So, I guess adding a nice error about missing mypy for the plugin code + removing it from the required requirements is not a big problem.

@jorenham
Copy link
Contributor Author

@sobolevn Why do you think that most people have mypy in their requirements? And do you think that this is very much dependent on the fact that django-stubs requires mypy?

Also, take into account that the most popular free python IDE, vscode, has seamless pylance (i.e. pyright) integration, whereas mypy integration is very limited and clunky in my experience (pycharm and vscode).
For most new typing-related PEP's, mypy is the slowest in adopting them.

I'm not trying to promote one type checker over another, but it's things like these that users consider when choosing/switching (static) type checkers.

Given the current python-typing ecosystem, with typing definitions that are very prone to interpretation (it kinda reminds me of the first HTML5 specs), I don't think that limiting a stubs package to one single typechecker is a good idea.

@sobolevn
Copy link
Member

sobolevn commented Jul 17, 2023

@jorenham this is not what I meant :)

I mean: if people need mypy and django-stubs they probably have both in their requirements. That's it.

If you don't need mypy, that's fine: we can make it optional.

@jorenham
Copy link
Contributor Author

@sobolevn Ah ok, sorry for the confusion

@adamchainz
Copy link
Contributor

I am fine with dropping the hard dependency but I don't think we should claim any level of support, maybe even add a disclaimer. Other type checkers won't run the plugin and may interpret more complex types differently.

@sobolevn
Copy link
Member

If anyone wants to contribute better pyright support with CI, tests, configuration, and fixes – I will be more than happy to review.

But, I don't use pyright myself :)

@jorenham
Copy link
Contributor Author

@sobolevn I'll give it a shot in the coming days

@jorenham
Copy link
Contributor Author

Pyright with typeCheckingMode = "basic" and include = ["django-stubs", "django_stubs_ext"], reports:

./django-stubs/conf/__init__.pyi
  ./django-stubs/conf/__init__.pyi:39:17 - warning: __new__ override should take a "cls" parameter (reportSelfClsParameterName)
  ./django-stubs/conf/__init__.pyi:7:15 - warning: Import "global_settings" is not accessed (reportUnusedImport)
./django-stubs/contrib/admin/helpers.pyi
  ./django-stubs/contrib/admin/helpers.pyi:181:28 - error: "utils" is not a known member of module "django.contrib.admin.helpers.forms" (reportGeneralTypeIssues)
./django-stubs/contrib/admin/views/decorators.pyi
  ./django-stubs/contrib/admin/views/decorators.pyi:7:38 - warning: Type variable "_C" may go unsolved if caller supplies no argument for parameter "view_func"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
./django-stubs/contrib/contenttypes/forms.pyi
  ./django-stubs/contrib/contenttypes/forms.pyi:30:16 - warning: Type variable "_ModelFormT" may go unsolved if caller supplies no argument for parameter "form"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
./django-stubs/core/paginator.pyi
  ./django-stubs/core/paginator.pyi:13:7 - error: Cannot create consistent method ordering
./django-stubs/db/models/manager.pyi
  ./django-stubs/db/models/manager.pyi:10:30 - error: "ValuesQuerySet" is unknown import symbol (reportGeneralTypeIssues)
./django-stubs/db/models/query.pyi
  ./django-stubs/db/models/query.pyi:59:49 - error: Covariant type variable cannot be used in parameter type (reportGeneralTypeIssues)
./django-stubs/forms/models.pyi
  ./django-stubs/forms/models.pyi:90:19 - warning: Type variable "_M" may go unsolved if caller supplies no argument for parameter "instance"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/forms/models.pyi:129:28 - warning: Type variable "_M" may go unsolved if caller supplies no argument for parameter "queryset"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/forms/models.pyi:156:16 - warning: Type variable "_ModelFormT" may go unsolved if caller supplies no argument for parameter "form"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/forms/models.pyi:189:19 - warning: Type variable "_ParentM" may go unsolved if caller supplies no argument for parameter "instance"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/forms/models.pyi:192:28 - warning: Type variable "_M" may go unsolved if caller supplies no argument for parameter "queryset"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/forms/models.pyi:205:16 - warning: Type variable "_ModelFormT" may go unsolved if caller supplies no argument for parameter "form"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
./django-stubs/http/request.pyi
  ./django-stubs/http/request.pyi:175:56 - warning: TypeVar "_Z" appears only once in generic function signature (reportInvalidTypeVarUse)
./django-stubs/http/response.pyi
  ./django-stubs/http/response.pyi:25:14 - error: Variable not allowed in type expression (reportGeneralTypeIssues)
./django-stubs/test/client.pyi
  ./django-stubs/test/client.pyi:71:14 - error: Variable not allowed in type expression (reportGeneralTypeIssues)
./django-stubs/test/testcases.pyi
  ./django-stubs/test/testcases.pyi:11:23 - warning: Import "connections" is not accessed (reportUnusedImport)
./django-stubs/utils/datastructures.pyi
  ./django-stubs/utils/datastructures.pyi:38:7 - error: Cannot create consistent method ordering
  ./django-stubs/utils/datastructures.pyi:46:43 - warning: Type variable "_K" may go unsolved if caller supplies no argument for parameter "iterable"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/utils/datastructures.pyi:60:53 - warning: Type variable "_K" may go unsolved if caller supplies no argument for parameter "key_to_list_mapping"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/utils/datastructures.pyi:60:62 - warning: Type variable "_V" may go unsolved if caller supplies no argument for parameter "key_to_list_mapping"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/utils/datastructures.pyi:62:60 - warning: Type variable "_K" may go unsolved if caller supplies no argument for parameter "key_to_list_mapping"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
  ./django-stubs/utils/datastructures.pyi:62:69 - warning: Type variable "_V" may go unsolved if caller supplies no argument for parameter "key_to_list_mapping"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
./django-stubs/utils/functional.pyi
  ./django-stubs/utils/functional.pyi:101:48 - warning: Type variable "_Get" may go unsolved if caller supplies no argument for parameter "method"
    Provide an overload that specifies the return type when the argument is not supplied (reportInvalidTypeVarUse)
./django-stubs/views/decorators/csrf.pyi
  ./django-stubs/views/decorators/csrf.pyi:6:25 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/csrf.pyi:6:30 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/csrf.pyi:10:32 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/csrf.pyi:10:37 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/csrf.pyi:14:31 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/csrf.pyi:14:36 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
./django-stubs/views/decorators/gzip.pyi
  ./django-stubs/views/decorators/gzip.pyi:6:22 - error: Type variable "_C" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/gzip.pyi:6:27 - error: Type variable "_C" has no meaning in this context (reportGeneralTypeIssues)
./django-stubs/views/decorators/http.pyi
  ./django-stubs/views/decorators/http.pyi:7:29 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:7:34 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:11:24 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:11:29 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:12:25 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:12:30 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:13:25 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
  ./django-stubs/views/decorators/http.pyi:13:30 - error: Type variable "_F" has no meaning in this context (reportGeneralTypeIssues)
./django_stubs_ext/django_stubs_ext/patch.py
  ./django_stubs_ext/django_stubs_ext/patch.py:95:18 - error: "reveal_type" is not a known member of module "builtins" (reportGeneralTypeIssues)
  ./django_stubs_ext/django_stubs_ext/patch.py:96:18 - error: "reveal_locals" is not a known member of module "builtins" (reportGeneralTypeIssues)
./django_stubs_ext/django_stubs_ext/db/models.py
  ./django_stubs_ext/django_stubs_ext/db/models.py:8:34 - error: "StrOrPromise" is unknown import symbol (reportGeneralTypeIssues)
26 errors, 18 warnings, 0 informations 

In strict mode (with typeCheckingMode = "strict" and reportPrivateUsage = false), 1023 errors are reported 😅 . Missing type arguments make up 662 of these, e.g. bare spam: Callable annotations. I suspect that a significant amount of the other errors are a consequence of these missing type arguments.

@last-partizan
Copy link

Also note that we have https://github.com/sbdchd/django-types by @sbdchd, which was forked some time ago with the goal of removing mypy dependency.

And maybe some of that can be merged back here to improve compatibility.

@intgr
Copy link
Collaborator

intgr commented Oct 13, 2023

Cross-posting from #1740 (comment)

I want to move forward with this.

Making no changes quite yet, but I propose mentioning in 4.2.5 release notes that the next next version (4.2.6) will remove hard mypy dependency. Something like:

django-stubs is taking steps to improve experience with other type checkers besides mypy. Next version (4.2.6) will remove explicit mypy dependency. If you are currently using mypy, please add an explicit dev dependency on mypy to your project, or depend on django-stubs with the compatble-mypy extra, e.g. django-stubs[compatible-mypy].
Note: this is not a guarantee that other type checkers will be fully supported, pull requests for improvements are welcome. See #1628 for details.

What do you think?

@sobolevn
Copy link
Member

I would like to note that the future of "other type-checkers" right now are generally in question. I think we should really wait to see where all of this is going. I really hope that the eco-system will still be stable, but I don't want to promise things that might be affected by 3rd parties.

@intgr
Copy link
Collaborator

intgr commented Oct 13, 2023

I would like to note that the future of "other type-checkers" right now are generally in question. I think we should really wait to see where all of this is going.

My understanding from reading this thread is that we have consensus for removing the dependency on mypy. Or did you reconsider that?

Or did you mean that I should adjust the phrasing of my proposed release notes notice to lower expectations?

@sobolevn
Copy link
Member

Yes, let's lower expectations for now :)

@intgr
Copy link
Collaborator

intgr commented Oct 13, 2023

OK, I also tried to make the message shorter. Any feedback on this one:

Next django-stubs version (4.2.6) will remove direct mypy dependency. If you are using mypy, please add an explicit mypy dev dependency to your project, or install django-stubs with the extra django-stubs[compatible-mypy].

Mypy remains the only supported type checker. Support for other type checkers may be considered in the future, pull requests welcome. See #1628 for details.

@intgr
Copy link
Collaborator

intgr commented Oct 13, 2023

Maybe instead of linking to this issue, we should create a new placeholder issue, which better describes our stance on other type checkers and lists expected requirements for supporting additional type checkers? (if we can imagine what those requirements are).

I'm guessing CI integration is a strict requirement, anything else?

Or maybe I'm just getting ahead of myself here...

@intgr
Copy link
Collaborator

intgr commented Oct 13, 2023

Maybe "support" is too strong a word to use...

-Support for other type checkers may be considered in the future, pull requests welcome.
+Improvements for other type checkers may be considered in the future, pull requests welcome.

Best to start with small improvements, without committing to anything initially.

@adamchainz
Copy link
Contributor

Small improvements, yes, let’s just open the door a bit to see how feasible support feels before we commit to maintaining long term.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request meta Meta-issues and discussion pyright Related to pyright type checker
Development

Successfully merging a pull request may close this issue.

6 participants