Consider choices as literal and a lot more typing improvements

[bellini666]

Ok, this started with me trying to make choices be automatically considered for typing validation. E.g.:

class Foo(models.Model):
    c = models.CharField(choices=[("a": "A"), ("b": "B")])

f = Foo()
reveal_type(c)  # would display Literal["a", "b"]
f.c = "c"  # error

It worked very fine! I just had to implement new for both IntegerField and CharField to make it work.

Then, since with the lack of __init__ I wasn't getting arguments completion anymore, I implemented it generically on Field. I had to manually defined those on related fields and also on ArrayField, which I ended up improving their typing.

If you like this approach, it will be nice to do the same for all other fields (I can do that, just didn't because wasn't sure you were gonna like it).

Also, I tried a lot to avoid having to overload new 4 times in each field but coudn't find a way :(. Maybe I'm missing something after looking to the same code for hours...

Fix #53 Fix #51 Fix #34

[bellini666]

@sbdchd so, here is a summary of everything and some considerations:

1) I added typing to all models/fields.py, related.py, array.py and fields/json.py

2) All the will be considered as Literal[A, B, C, ...] if there are choices included in them. Also, null=True/False is considered for all the cases just like before.

3) I improved a place or other where the types where missing, the signatures where wrong or missing, etc. E.g. although max_digits and decimal_places are defined in the decimal field's __init__ as =None, they are checked at runtime and will give an error if not defined. So, I decided to make them mandatory so the typing can help us in there

4) The update to pyright is still here. If its PR is merged its diff will probably vanish from the PR (although it is pretty small)

5) The changes to fields/__init__.pyi is pretty big, but if you look how one field is doing it you will notice that everything else is just ctrl+c/ctrl+v with adjustments to the name of the class and the TypeVar itself. Some override the init because they have extra arguments (e.g. the decimal that I mentioned above). If they don't have a different signature from the base Field, then they will only define 4 new overloads

Now, some considerations regarding some things that might not be obvious:

1) You will notice that I changed most of the setters of the fields to reduce the number of types they accept. For example, IntegerField said it accepted int | float | decimal | str. Even though the django field will actually accept those, IMO the typing should help us prevent errors, and thus, not warning about model.int_field = "xxx" I consider to be a bad thing.

I think that, in those cases, if you are sure about that, you can either int(value) or cast(int, value). Removing those other possibilities from the setters will help reduce coding errors in my option.

This is pretty related to the reason why I started this change. I wanted to make sure that, when choices are defined, the field itself would say that it returns a Literal of those choices, and only accept those too.

One might say that float and decimal maybe could be accepted here because they are somewhat "subclasses of int" and they are totally safe to be converted to int. Although I agree with this info, like I said before, I believe that this can lead to errors like one setting a decimal/float to this field and loosing the number's decimal places. Like I mentioned above, I think it is good that the typing will alert in those cases, and the user can choose to ignore it with or even fix it (if it was a mistake in the first place)

2) The JSONField needs to be typed when in strict mode. If I used the same method as I did with the rest, for example, a non-typed IntegerField will be Integer[int], but json would be JSONField[Any], and since Any matches anything, I could not do JSONField[dict[str, str]] for example as it would match Any either way.

I also think this is a good thing as it helps typing something that is in nature Any. So, someone wanting to use this can just json_field = JSONField[Any](), but one can json_field = JSONField[dict[str, list[int]]]() for example and have it properly typed, and also act like a schema validator in the typing itself.

3) The unreachable code that you mentioned. After spending a lot of time in that, I could not for the world make it say the code is unreachable. I added a FIXME to the code with the reveal_type() output of mypy, which says the type is the expected one and thus, if not <> should be unreachable.

I even tried the following:

    a: List[List[str]] = [["str"]]
    if not isinstance(a, list):
        print()  # type: ignore [unreachable]

This is the same as what should return from comment.array but very explicitly typed. mypy gave me the the same error: unused 'type: ignore' comment. So, if that code didn't consider the code to be unreachable, I'm very confident to say that this is mypy's issue.

[bellini666]

Also, I just rebased from main branch since the pyright PR was merged (hope you weren't using any of the commits from here =P)

[bellini666]

Hey @sbdchd , did you have the opportunity to look at this? I know it is a very large PR with a lot of implications, but feel free to tell me if you need some better explanation or even something changed

[sbdchd]

Sorry for the delay!

I think right now it's tricky to review since it's so large, probably easier if its done in chunks

Also I want to support both mypy and pyright with this project. I tend to use pyright for intellisense and then mypy for actual type checking

[bellini666]

Hey @sbdchd , well, I can try to split this a bit if it helps. As I mentioned in this comment 90% of the changes are actually pretty similar, it is just that there are lots of overloads that had to be added to all Field subclasses. There are some improvements to some typings, like the ManyToMany and ArrayField being able to resolve their types and also JSONField being able to receive a type to properly describe it.

Regarding mypy, it is actually working and the types are correct for it. The only strange thing is this. I really think that is a bug in mypy specifically with that unreachable code. I say that because, when I ask it to reveal_type, it says that it is indeed that type, it is not confused regarding it. Both it and pyright are reporting them being the same and also correctly.

[bellini666]

Hey @sbdchd ,

I just separated this PR into 5:

1) https://github.com/sbdchd/django-types/pull/62 : This is the first one that can be merged. I updated black/flake8 and installed flake8-pyi and fixed all related issues. All the bellow depends on this one

2) https://github.com/sbdchd/django-types/pull/63 : This improves RelatedFields and ManyToMany to properly type it.

3) https://github.com/sbdchd/django-types/pull/64 : This improves JSONField and allows us to type it and it will get validated. This can be useful to pass a TypedDict for example to define the JSONField schema.

4) https://github.com/sbdchd/django-types/pull/65 : This is the main change of this issue. It improves typing for all fields in fields/__init__.pyi. Note that it depends on the others being merged first

5) https://github.com/sbdchd/django-types/pull/66 : This improves typing for arrayfields. Note that it depends on 4) being merged.

It should be safe to close this PR as the changes are separated into each one

[bellini666]

I'm closing this as it was splitted into many other PRs.