Skip to content

Feature: Use PEP 696 defaults for fields #1264

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

Open
w0rp opened this issue Nov 16, 2022 · 11 comments
Open

Feature: Use PEP 696 defaults for fields #1264

w0rp opened this issue Nov 16, 2022 · 11 comments
Labels
enhancement New feature or request pyright Related to pyright type checker

Comments

@w0rp
Copy link
Contributor

w0rp commented Nov 16, 2022

NOTE: This feature can't be worked on unless PEP 696 is accepted, but given its inclusion in typing_extensions I'd say it's likely it will be.

django-stubs uses set and get value generic types for fields, defined like so:

# __set__ value type
_ST = TypeVar("_ST", contravariant=True)
# __get__ return type
_GT = TypeVar("_GT", covariant=True)

class Field(RegisterLookupMixin, Generic[_ST, _GT]):

Specific field types use special attributes that the mypy plugin for django-stubs reads to fill in the get and set types with.

class CharField(Field[_ST, _GT]):
    _pyi_private_set_type: Union[str, int, Combinable]
    _pyi_private_get_type: str
    
    ...

This works for mypy, but not for other type checkers, where the generic parameters need to be written manually. To better support different type checkers, we could use the default values from PEP 696 instead.

_ST_CHAR = TypeVar("_ST_CHAR", contravariant=True, default=Union[str, int, Combinable])
_GT_CHAR = TypeVar("_GT_CHAR", covariant=True, default=str)

class CharField(Generic[_ST_CHAR, _GT_CHAR], Field[_ST_CHAR, _GT_CHAR]):
    ...

The mypy plugin could read the default values instead of the _pyi_private* attributes from before. For other type checkers, the instance values for __get__ and __set__ will then use the generic type defaults if not specified otherwise.

@intgr
Copy link
Collaborator

intgr commented Mar 7, 2023

This feature can't be worked on unless PEP 696 is accepted

I think we don't necessarily have to wait for acceptance of the PEP. As long as it's implemented in typing_extensions, there is at least 1 type checker implementation that already supports it, and its adoption does not cause regressions for other type checkers (particularly mypy), I would be in support for merging this.

Once PEP 696 is implemented in mypy itself (python/mypy#14851), maybe we can get rid of this logic in the mypy plugin? Native type constructs tend to be less fragile and better supported in general.

@w0rp
Copy link
Contributor Author

w0rp commented Mar 8, 2023

Thanks for the comment. Generally I would like django-stubs to be a bit more useful for tools like Pyright by default without making life harder for people who benefit from the mypy plugin.

@intgr intgr added pyright Related to pyright type checker enhancement New feature or request labels Nov 9, 2023
@christianbundy
Copy link
Contributor

Would this also provide the ability to have generic fields? For example:

class CharField(Field[_ST, _GT]):
    _pyi_private_set_type: _ST
    _pyi_private_get_type: _GT

My use-case: I have a custom field class (a subclass of TextField) that transforms data on its way to/from the database, but it doesn't seem to me that django-stubs supports any way to have generic setter/getter types via _pyi_private_{set,get}_type.

@flaeppe
Copy link
Member

flaeppe commented Dec 13, 2023

My current approach for custom fields is to cast them like e.g.

class SomeModel(models.Model):
    currency = cast(
        "models.CharField[Currency, Currency]",
        CurrencyField(default=None, verbose_name=_("currency")),
    )

It gets quite verbose though, but at least it works.

@Viicos
Copy link
Contributor

Viicos commented Dec 18, 2023

This works for mypy, but not for other type checkers, where the generic parameters need to be written manually. To better support different type checkers, we could use the default values from PEP 696 instead.

_ST_CHAR = TypeVar("_ST_CHAR", contravariant=True, default=Union[str, int, Combinable])
_GT_CHAR = TypeVar("_GT_CHAR", covariant=True, default=str)

class CharField(Generic[_ST_CHAR, _GT_CHAR], Field[_ST_CHAR, _GT_CHAR]):
    ...

One issue with this is taking into account null, that when set to True allow _ST and _GT to be None. Instead of using type var defaults, this could be achieved with:

class Field(Generic[_ST, _GT]):
    @overload
    def __new__(
        cls,
        *,
        null: Literal[True],
    ) -> Field[_ST | None, _GT | None]: ...
    @overload
    def __new__(
        cls,
        *,
        null: Literal[False] = ...,
    ) -> Field[_ST, _GT]: ...

Then field subclasses can be written as:

class IntegerField(Field[float | int | str | Combinable, int]):
    ...

Working as expected:

from typing import Generic, TypeVar, Literal, overload


_ST = TypeVar("_ST")
_GT = TypeVar("_GT")


class Field(Generic[_ST, _GT]):
    @overload
    def __new__(
        cls,
        *,
        null: Literal[True],
    ) -> Field[_ST | None, _GT | None]: ...
    @overload
    def __new__(
        cls,
        *,
        null: Literal[False] = ...,
    ) -> Field[_ST, _GT]: ...

    def get_type(self) -> _GT: ...
    def set_type(self) -> _ST: ...

class IntegerField(Field[float | int | str, int]):
    pass


a = IntegerField(null=False)
reveal_type(a.get_type())  # Type of "a.get()" is "int"   + None if null=True
reveal_type(a.set_type())  # Type of "a.set()" is "float | int | str" + None if null=True

However, this requires two overloads each time a field is defined with a new signature (IntegerField is fine here, but this is not the case for all fields available in the stubs).

Edit: see https://discuss.python.org/t/41483, this is in fact not really possible.

@finlayacourt
Copy link

I think it’s possible to get nullability working by

  • defining a new type var that represents nullability
  • defining additional overloads for the __set__ and __get__ methods in Field

Then we don't have to add overloads to each field type.

Here's a minimal example of just the Field and IntegerField types

_ST = TypeVar("_ST", contravariant=True)
_GT = TypeVar("_GT", covariant=True)
_NT = TypeVar("_NT", Literal[True], Literal[False], default=Literal[False])

class Field(RegisterLookupMixin, Generic[_ST, _GT, _NT]):
    def __init__(self, null: _NT = False) -> None: ...
    @overload
    def __set__(self: Field[_ST, _GT, Literal[False]], instance: Any, value: _ST) -> None: ...
    @overload
    def __set__(self: Field[_ST, _GT, Literal[True]], instance: Any, value: _ST | None) -> None: ...
    @overload
    def __get__(self, instance: None, owner: Any) -> _FieldDescriptor[Self]: ...
    @overload
    def __get__(self: Field[_ST, _GT, Literal[False]], instance: Model, owner: Any) -> _GT: ...
    @overload
    def __get__(self: Field[_ST, _GT, Literal[True]], instance: Model, owner: Any) -> _GT | None: ...
    @overload
    def __get__(self, instance: Any, owner: Any) -> Self: ...

_ST_1 = TypeVar("_ST_1", bound=float | int | str | Combinable, default=float | int | str | Combinable)
_GT_1 = TypeVar("_GT_1", bound=int, default=int)

class IntegerField(Field[_ST_1, _GT_1, _NT]): ...

@Viicos
Copy link
Contributor

Viicos commented May 8, 2025

@finlayacourt this isn't compatible yet with mypy, which doesn't allow (yet) default values to be assigned to type variables. I'm on mobile so can't find links, but you might find more information in one of my linked PR/issue here.

@finlayacourt
Copy link

Ah I see so the def __init__(self, null: _NT = False) -> None: ... above errors in mypy?

Regardless, it looks like mypy doesn’t understand the overloads unless they are redefined in every field type. See how in the example below IntegerField2 has correct types, but IntegerField1 does not.

from typing import TypeVar, Literal, Generic, overload, Any, reveal_type, Self, Never

_ST = TypeVar("_ST", contravariant=True, default=Any)
_GT = TypeVar("_GT", covariant=True, default=Any)
_NT = TypeVar("_NT", Literal[True], Literal[False], default=Literal[False])

class Model:
    pass

class Field(Generic[_ST, _GT, _NT]):
    def __init__(self, null: _NT) -> None: ...
    @overload
    def __get__(self: Field[Any, Any, Literal[False]], instance: Any, owner: Any) -> _GT: ...
    @overload
    def __get__(self: Field[Any, Any, Literal[True]], instance: Any, owner: Any) -> _GT | None: ...
    def __get__(self, instance, owner):
        pass

_ST_1 = TypeVar("_ST_1", bound=int, default=int)
_GT_1 = TypeVar("_GT_1", bound=int, default=int)

class IntegerField1(Field[_ST_1, _GT_1, _NT]): ...

class IntegerField2(Field[_ST_1, _GT_1, _NT]):
    @overload
    def __get__(self: IntegerField2[Any, Any, Literal[False]], instance: Any, owner: Any) -> _GT_1: ...
    @overload
    def __get__(self: IntegerField2[Any, Any, Literal[True]], instance: Any, owner: Any) -> _GT_1 | None: ...
    def __get__(self, instance, owner):
        pass

class A(Model):
    field1 = IntegerField1(null=False)
    field2 = IntegerField1(null=True)
    field3 = IntegerField2(null=False)
    field4 = IntegerField2(null=True)

a = A()
reveal_type(a.field1) # Revealed type is "builtins.int"
reveal_type(a.field2) # Revealed type is "builtins.int" <- INCORRECT
reveal_type(a.field3) # Revealed type is "builtins.int"
reveal_type(a.field4) # Revealed type is "Union[builtins.int, None]" <- CORRECT

https://mypy-play.net/?mypy=latest&python=3.13&gist=37d176d6857fdf972f74294cebf0245a

@UnknownPlatypus
Copy link
Contributor

pyright seem to get it right with this approach

@Viicos
Copy link
Contributor

Viicos commented May 9, 2025

Yes, I think you basically have the same approach I was trying to get in microsoft/pyright#6914, which is great as you don't need to define overloads on every field subclass as I did in #1900.

@finlayacourt
Copy link

Intrestingly mypy infers the correct type when __get__ is called directly. Using the same setup as from above:

reveal_type(IntegerField1(null=False).__get__(None, None)) # Revealed type is "builtins.int"
reveal_type(IntegerField1(null=True).__get__(None, None)) # Revealed type is "Union[builtins.int, None]"

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

No branches or pull requests

7 participants