django-stubs
django-stubs copied to clipboard
Proposal: Allow narrowing ORM fields
I am currently working on implementing phantom types for Python, which, when implemented throughout an application, would allow statically proving that a value saved on model is validated.
To be able to use this with django-stubs there would need to be some way of narrowing the types that ORM fields hold.
One example would be a field for country codes:
from phantom.ext.iso3166 import CountryCode
class Member(models.Model):
name = CharField()
country: CountryCode = CharField(max_length=2)
This makes mypy error if I try to assign a string value to the field:
m = Member()
m.country = "SE" # E: Incompatible types in assignment (expression has type "str", variable has type "CountryCode")
Unless I first prove that the string is a valid country code:
country_code = "SE"
assert isinstance(country_code, CountryCode)
m.country = country_code
# ... or
m.country = CountryCode.parse(country_code)
Unfortunately mypy raises an error when annotating ORM fields:
Incompatible types in assignment (expression has type "CharField[Union[str, int, Combinable, None], Optional[str]]", variable has type "CountryCode") [assignment]
Although my usecase is phantom types, I believe a feature that enables this would also work equally well with typing.NewType
types.
This can be worked around with using the usual dirty tricks of ignore comments or using e.g. typing.cast(CountryCode, CharField())
. But it would be very nice to see builtin support for this. I think it would be optimal if this builtin support also verifies that the annotated type is a subtype of the type that the field holds, so that the type annotated to a CharField
must be a subtype of str
and so on.
What do you think?
PS.: This is a proposal to get a discussion started, I'm not asking anyone to implement this for me.
Hi, @antonagestam!
You can use this notation:
score: 'models.Field[int, int]' = models.IntegerField(default=0)
Will it work for you?
The problem is that you currently can specify wrong generic params:
score: 'models.Field[str, str]' = models.IntegerField(default=0) # wrong, but typechecks!
@sobolevn Cool, I did not know about that! I think I'm running into a variance issue though.
I setup a minimal Django project to test with, and testing with this definition just to have less moving parts to think about (also tested with NewType
with same results):
from __future__ import annotations
from django.db import models
class CountryCode(str):
pass
class Member(models.Model):
name = models.CharField(max_length=200)
country: models.Field[CountryCode, CountryCode] = models.CharField(max_length=2)
I get:
phango/models.py:12: error: Incompatible types in assignment (expression has type "CharField[Union[str, int, Combinable], str]", variable has type "Field[CountryCode, CountryCode]") [assignment]
country: models.Field[CountryCode, CountryCode] = models.CharField(max_length=2)
^
If I change the definitions of _ST
and _GT
that Field
is generic with regards to, to be contravariant I seem to get the expected behavior without errors. I can open a PR introducing this change.
One further issue though, that might be unrelated to this at all is that I don't get type errors passing field values when instantiating the model. I'm assuming this is unrelated because it passes even with this example:
from __future__ import annotations
from django.db import models
class Member(models.Model):
value: models.Field[int, int] = models.IntegerField()
m = Member(value="test") # Expected error here
@antonagestam I've been trying to get to the bottom of some similar issues and I don't think that _ST
and _GT
should both be contravariant. I think that _GT
should be covariant instead. This would mean that in practice the Field
type is type invariant, but you should still be able to get some better typing when getting and setting field values.
Say we have a phantom NegativeInteger
type, which is a subtype of int
, and we have the models:
from __future__ import annotations
from django.db import models
class IntMember(models.Model):
value: models.Field[int, int] = models.IntegerField()
class NegativeIntMember(models.Model):
value: models.Field[NegativeInt, NegativeInt] = models.IntegerField()
The field should be contravariant when you set it, as IntMember
is effectively a subclass of NegativeIntMember
for the purposes of setting value
. This would result in:
def set_value_to_minus_ten(instance: NegativeIntMember):
instance.value = NegativeInt(-10)
def set_value_to_fifteen(instance: IntMember):
instance.value = 15
set_value_to_minus_ten(IntMember(value=7)) # This is fine
set_value_to_minus_ten(NegativeIntMember(value=NegativeInt(-20)) # This is also fine
set_value_to_fifteen(IntMember(value=7)) # This is fine
set_value_to_fifteen(NegativeIntMember(value=NegativeInt(-20)) # This should raise an error
but it should be covariant when you get it, as NegativeIntMember
is effectively a subclass of IntMember
when you are getting value
:
int_member = IntMember(value=7)
negative_member = NegativeIntMember(value=NegativeInt(-20))
int_value: int = int_member.value # This is fine
negative_value: int = negative_member.value # Also fine
negative_value: NegativeInt = negative_member.value # Also fine
int_value: NegativeInt = int_member.value # This should raise an error
@leamingrad this looks like a valid proposal! Do you have the time / will to submit an experiment PR?
@leamingrad Ah, I didn't realize they should be reversed to each other. Very nicely explained! 👍
(Sorry for off topic): Feel free to jump in on phantom-types/discussions if you'd feel like sharing how you use phantom types! We've slowly started to use them in a production project at work now, would be nice with feedback from other users too :)
I created a new PR: https://github.com/typeddjango/django-stubs/pull/573
It does make it possible to narrow a field's type, but I found no other way to do that than to use cast()
. I think that's related to the plugin's method of picking it up from _pyi_private_{set,get}_type
. It should probably only do that if the arguments aren't explicitly overridden?
@antonagestam I guess so, but it should still check that types are defined corretly. Maybe we can incorporate this change into your PR as well?
@sobolevn Yes, fixing that would be awesome. I started looking into it a bit but I somehow always find it extremely hard to understand what's going on in mypy plugins. I assume changes have to be made in https://github.com/typeddjango/django-stubs/blob/master/mypy_django_plugin/transformers/fields.py, but I would need some help to figure out how get types from explicit arguments when provided.
These are the results I get when not using cast()
:
Both these give the error below, the first one probably makes sense, but the second one is because the explicit arguments aren't picked up.
published: models.Field[Year, Year] = models.IntegerField()
published: models.Field[Year, Year] = models.IntegerField[Year, Year]()
main:6: error: Incompatible types in assignment (expression has type "IntegerField[Union[float, int, str, Combinable], int]", variable has type "Field[Year, Year]") (diff)
Also tried these:
published: models.IntegerField[Year, Year] = models.IntegerField()
published: models.IntegerField[Year, Year] = models.IntegerField[Year, Year]()
main:7: error: Incompatible types in assignment (expression has type "IntegerField[Union[float, int, str, Combinable], int]", variable has type "IntegerField[Year, Year]") (diff)
This doesn't give an error at assignment, but reveal_type()
doesn't give [Year, Year]
:
published = models.IntegerField[Year, Year]()
Revealed type is 'django.db.models.fields.IntegerField[Union[builtins.float, builtins.str, django.db.models.expressions.Combinable], builtins.int]' (diff)
Yes, looks like we need to fix this in our plugin.
I assume changes have to be made in https://github.com/typeddjango/django-stubs/blob/master/mypy_django_plugin/transformers/fields.py
Yes, I would start from here: https://github.com/typeddjango/django-stubs/blob/caaa23ab8fd4bd978fd0bf6821b1c7db45861c1c/mypy_django_plugin/main.py#L208-L210
but I would need some help to figure out how get types from explicit arguments when provided
Sure, please feel free to ask!
I am going to leave this open, since we can also change how our plugin works.
@antonagestam Thanks for getting a PR merged so quickly! To be honest, I don't actually use phantom types, but it seemed like a clearer way to give examples (most of the issues I have been working with are to do with relations between abstract models which this is a precursor of). That said, it seems like an interesting idea so I will definitely check it out.
I don't actually use phantom types
This is pure gold, @antonagestam I am marketing it a lot 🙂
Hi, I'm trying to add types to django-phonenumber-field package, and it turned out trickier than expected.
I've got it working locally with django-types
and pyright
, by using following stubs for django-phonenumber
class PhoneNumberField(models.CharField[_T]): # type: ignore
@overload
def __new__(
cls, *args, null: Literal[False] = ..., **kwargs
) -> PhoneNumberField[PhoneNumber]: ...
@overload
def __new__(
cls, *args, null: Literal[True] = ..., **kwargs
) -> PhoneNumberField[PhoneNumber | None]: ...
@overload
def __new__(cls, *args, **kwargs) -> PhoneNumberField[PhoneNumber]: ...
def __new__(cls, *args, **kwargs) -> PhoneNumberField[PhoneNumber | None]: ...
And now i want to make proper PR for the project, but we decided to use mypy and django-stubs, and i cannot figure out how to properly assign types.
My first try was to use class PhoneNumberField(CharField[PhoneNumber | str, PhoneNumber])
, but it doesn't work.
Then i tried using _pyi_private_set_type:
, and it not working either.
tests/models.py:3: note: In module imported here:
phonenumber_field/modelfields.py: note: In class "PhoneNumberField":
phonenumber_field/modelfields.py:53: error: Incompatible types in assignment (expression has type
"Union[str, PhoneNumber, Combinable]", base class "CharField" defined the type as "Union[str, int, Combinable]")
[assignment]
_pyi_private_set_type: str | PhoneNumber | Combinable
^
phonenumber_field/modelfields.py:54: error: Incompatible types in assignment (expression has type "PhoneNumber", base
class "CharField" defined the type as "str") [assignment]
_pyi_private_get_type: PhoneNumber
^
tests/tests.py: note: In member "test_typing" of class "PhoneNumberFieldAppTest":
tests/tests.py:298: note: Revealed type is "Any"
Found 2 errors in 1 file (checked 9 source files)
Any ideas how to do this?