Skip to content

Django 3.1 JSONField is missing #439

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

Closed
bespokebob opened this issue Aug 5, 2020 · 25 comments
Closed

Django 3.1 JSONField is missing #439

bespokebob opened this issue Aug 5, 2020 · 25 comments
Labels
bug Something isn't working

Comments

@bespokebob
Copy link

Bug report

What's wrong

Django 3.1 adds a generic models.JSONField and fields.JSONField to replace the PostgreSQL-only versions:
https://docs.djangoproject.com/en/3.1/releases/3.1/#jsonfield-for-all-supported-database-backends

Attempting to use these new fields results in error: Module has no attribute "JSONField"

How is that should be

Type definitions should exist for the new imports. These can probably just be copied from the existing PostgreSQL versions.

System information

  • OS:
  • python version: 3.8.5
  • django version: 3.1
  • mypy version: 0.770
  • django-stubs version: 1.5.0
@bespokebob bespokebob added the bug Something isn't working label Aug 5, 2020
@sobolevn
Copy link
Member

sobolevn commented Aug 5, 2020

Yeap, [email protected] is not yet supported.

@kszmigiel
Copy link
Member

@sobolevn I can take care of this in the meantime
@youngbob could you provide me some minimal reproduction code please?

@bespokebob
Copy link
Author

I was going to put together a PR since it seemed pretty straightforward, but I'm having issues running the tests 😞

A minimal case would be like:

from django.db import models

class JSONFieldModel(models.Model):
    json_field = models.JSONField()

Note that KeyTransform and KeyTextTransform should also be available from django.db.models.fields.json, like the PostgreSQL equivalent (just copying the existing type defs will probably work).
Source is here: https://github.com/django/django/blob/stable/3.1.x/django/db/models/fields/json.py

@sobolevn
Copy link
Member

sobolevn commented Aug 6, 2020

This is not that easy. Because we still don't know how to support multiple versions of django types in a single codebase.
Because versions <3.1 should not have these types.

@bespokebob
Copy link
Author

This is not that easy. Because we still don't know how to support multiple versions of django types in a single codebase.
Because versions <3.1 should not have these types.

What would be the impact of having the types available on <3.1? It seems like any code attempting to use the new fields would fail before type checking even came into play. If the types were changing in a backwards incompatible way, then it would be a problem, but I don't see what would break by making types available for things that just don't exist in older versions.

@federicobond
Copy link
Contributor

One way I have seen it done in other projects with similar constraints is to branch into two versions. 1.x could continue supporting the current versions while 2.x would support Django 3.1, and changes that apply to both can be backported to the 1.x branch.

An approach that is even easier to understand is to release versions that match the Django major and minor versions, with patch versions that follow incrementally from changes in django-stubs.

For example, the first version of django-stubs with Django 3.1 support would be 3.1.0. If any changes are done to it, then release 3.1.1, and so on. If any of those changes apply to previous versions, release them as 3.0.(n+1) and 2.2.(n+1) too.

The nice thing about this project is that most of the changes consist of declarations, which are relatively easy to backport. The main branch in any case should thus reflect the code for the highest supported version of Django, with separate branches supporting the previous versions.

@sobolevn
Copy link
Member

@federicobond yes, I guess it is the easiest way.

@bespokebob
Copy link
Author

An approach that is even easier to understand is to release versions that match the Django major and minor versions, with patch versions that follow incrementally from changes in django-stubs.

For example, the first version of django-stubs with Django 3.1 support would be 3.1.0. If any changes are done to it, then release 3.1.1, and so on. If any of those changes apply to previous versions, release them as 3.0.(n+1) and 2.2.(n+1) too.

That's basically the pattern used by the TypeScript definitions in https://github.com/DefinitelyTyped/DefinitelyTyped, and it works well.

@sobolevn
Copy link
Member

We need to think about how we should handle stubs and plugin. We cannot store our plugin in different branches.

@federicobond
Copy link
Contributor

@sobolevn can you clarify a bit more the requirements for the plugin and how they differ from the stubs?

One possible way forward would be to extract the plugin to a different repo.

@sobolevn
Copy link
Member

Plugin is not bound to django version. Stubs are.
We with @kszmigiel also think that moving our plugin to a separate repo is a good idea.

@relwell
Copy link

relwell commented Aug 21, 2020

Is there an approved workaround for this?

@kszmigiel
Copy link
Member

I've been thinking a while about this and here's my idea:

We could create one separate repo for stubs, having two main branches, for example master for 3.1 and newer, and legacy for pre-3.1. One of the branches should be pulled from the repo during install, in the similar way as we pull Django source code in scripts/typecheck_test.py at this moment.

Using pip functionality --install-option docs here we could give users availability to choose if they'd like to use pre-3.1 stubs, with 3.1 and higher as default (no --install-option used). So, the command for pre-3.1 would look like this:
pip install --install-option="--legacy" django-stubs

Though, it would require user to have gitpython package installed before installation starts, so adding it to dependencies is not enough. I'm not sure how we could resolve it.

BTW I think we should switch from distutils to setuptools in setup.py, shouldn't we?

@relwell
Copy link

relwell commented Aug 22, 2020 via email

@kszmigiel
Copy link
Member

@relwell Yes. The plugin itself is far from perfect, and user who would like to use Django Stubs with Django 2.2 would get a buggy plugin delivered, with no hope for improvement.

@relwell
Copy link

relwell commented Aug 23, 2020 via email

@kszmigiel
Copy link
Member

@relwell In that case if someone report an issue with plugin, we would have to apply it to either major and minor branch. This would generate twice as much PRs to review, and could cause a danger that plugin would work differently at major and minor version in case someone forgets to apply change to both branches. It's not about the stubs really, missing stub will generate an error, while bug in plugin might crash Mypy (or generate nonsense type annotations either).

@relwell
Copy link

relwell commented Aug 23, 2020 via email

@sobolevn
Copy link
Member

@kszmigiel let's discuss this tomorrow!

@niksubramanian
Copy link

Hi there,

I was wondering if there has been any update on this issue? I understand there's no perfect solution to this, but it'd be a shame if the last features of django aren't supported because of backwards compatibility issues...

Happy to help if I can!

@sobolevn
Copy link
Member

See #262

@kszmigiel
Copy link
Member

@niksubramanian Hello! Me and @sobolevn actually worked out a plan of making it work. We plan to divide stub files into separate repositories, as there is no simple solution to support various Django versions. It'll be done as soon as I finish my refactor, which lingers in time due to my excess of responsibilities. I'll get back to this, promise.

@niksubramanian
Copy link

@kszmigiel awesome! Thanks so much for all the hard work you're putting in the library - it has saved us countless hours of bug hunting, so it's very much appreciated.

@haplo
Copy link

haplo commented Jan 11, 2021

I experienced this bug when upgrading a project from Django 2.2 to Django 3.1 and I want to share my feelings and feedback.

I don't want to sound harsh but this makes me regret adding django-stubs and mypy to the project I'm working on, I wouldn't have done it if I had known about this beforehand. It's a project with several other programmers, with varying degrees of experience with Python, Django and MyPy, so I try to make the development experience as smooth as possible. Now if they use some Django 3.0 or 3.1-specific features such as JSONField they will see MyPy errors (that will need to be addressed as they are part of our pre-commit checks) and scratch their heads wondering where their mistake lies, when it's not their fault. I think that is an important loss in productivity that should be addressed.

I like @federicobond's idea of versioning the stubs to match the supported Django version, and backporting changes as necessary. However I recognize that might bee too cumbersome to maintain. In that case I like @kszmigiel's idea of having two branches:

  • lts or legacy branch to support LTS Django (2.2 currently).
  • master to support latest Django (3.1 currently).

I think the two-branch approach would cover all bases except users not on Django LTS and not on latest Django version. In my opinion those users could just pin django-stubs version to avoid upgrading until they upgrade Django.

Last comment was from October, can someone please give an update on whether this issue will be fixed or not, and if there is something I can do to help it get done?

Thank you for all your work on django-stubs.

liewegas added a commit to vote/puptime that referenced this issue Jan 12, 2021
...until it is added to stubs.  See typeddjango/django-stubs#439
mlissner added a commit to freelawproject/courtlistener that referenced this issue May 29, 2021
This is a bit annoying. Django says the old way is deprecated.
django-stubs doesn't know about the new way. This uses the new way
while ignoring the typing error.

See: typeddjango/django-stubs#439

Fixes: #1698
@adamchainz
Copy link
Contributor

Closing because JSONField was added in #467 and some fixes added in #558.

The problem of splitting types between Django versions is still unresolved, unfortunately, but it’s a known problem and I’ve discussed it recently with @sobolevn .

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

8 participants