False positive for argument mismatch

# pyright: reportGeneralTypeIssues=true
from django.db import models

class Test(models.Model):
    x = models.ForeignKey["Test"]("test.Test", models.CASCADE, null=True)
    y = models.ForeignKey("test.Test", models.CASCADE, null=True)

In this example, x = ... triggers following error in pyright:

  /tmp/test.py:6:69 - error: Argument of type "Literal[True]" cannot be assigned to parameter "null" of type "Literal[False]" in function "__init__"
    "Literal[True]" cannot be assigned to type "Literal[False]" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations

But i don't understand why, code is correct, and typing looks alright.

https://github.com/sbdchd/django-types/blob/main/django-stubs/db/models/fields/related.pyi#L93

Any ideas? Or does this looks like error in pyright?

Looks like problem is in

@override
def __init__

If i remove overrides and replace null: Literal[True] = ... with null: bool = ..., it works just fine.

And produces correct typing.

# pyright: reportGeneralTypeIssues=true
from django.db import models

class Test(models.Model):
    x = models.ForeignKey["Test"]("test.Test", models.CASCADE, null=True)
    y = models.ForeignKey("test.Test", models.CASCADE, null=True)

reveal_type(Test.x)
reveal_type(Test().x)

/tmp/test.py
  /tmp/test.py:9:13 - information: Type of "Test.x" is "ForwardManyToOneDescriptor"
  /tmp/test.py:10:13 - information: Type of "Test().x" is "Test | None"

I'll create PR for this change.

@last-partizan I think the issue here for X is that, when you define the typing in the field you need to consider the fact that it can be null, or else it fails to match the overload. More specifically, you should be writing it like this:

class Test(models.Model):
    x = models.ForeignKey[Optional["Test"]]("test.Test", models.CASCADE, null=True)

Hmm, well, it works this way and i'm currenctly using it as workaround.

But i think those overrides doing more harm than good, consider this:

class Test(models.Model):
    x = models.ForeignKey(Test, models.CASCADE, null=True)

It correctly resolves to Test | Null. And it could work the same for models.ForeignKey["Test"], if we remove overrides from __init__.

Folks at pyright are saying using types for self is undocumented feature of mypy, not found in any PEP. (https://github.com/microsoft/pyright/issues/2909#issuecomment-1019374993)

And i've raised bug report at mypy: https://github.com/python/mypy/issues/12045 , adding __init__ without type overrides breaks type inference from __new__, it clearly looks like a bug, and should be fixed.

And without those overrides code could be much cleaner.

It correctly resolves to Test | Null. And it could work the same for models.ForeignKey["Test"], if we remove overrides from init.

It resolves correctly because the overload is solving it for you. There's no overload possible where null=True and the model in the foreign key is not <Model> | None.

Folks at pyright are saying using types for self is undocumented feature of mypy, not found in any PEP. (microsoft/pyright#2909 (comment))

Yes, that's the reason of this issue: https://github.com/sbdchd/django-types/issues/87 . If we are doing overrides here, it is better to choose __init__ or __new__, not both.

Since typing self is an undocumented features and should not be relied on, the proper solution IMO is to do the overloading on __new__ only and avoid using __init__ at all. That means that in the current state we would need to overload all the arguments that the field accepts 4 times. I think it is better to remove the choices overload and have only 2, one for null=True and the other for null=False

I was thinking about using only __new__, but we're overloading it, and we require to repeat arguments two times. And I even tried it, but for some reason could not get it working as expected.

When we use overloads for __new__, with only arguments which are needed and __init__ for defining all arguments, code would be much cleaner.

It resolves correctly because the overload is solving it for you. There's no overload possible where null=True and the model in the foreign key is not <Model> | None.

It is possible, and already working as expected in pyright, but bug in mypy stops us from doing this right now. I think we should wait a bit for feedback from mypy folks.

@last-partizan yeah, that's the reason I've wrote it that way. That way you write the __init__ once, which in case of fields that don't have different arguments from the default you don't have to write it again, and you would just overload __new__ 4 times with only the required arguments for the overload itself.

It is possible, and already working as expected in pyright, but bug in mypy stops us from doing this right now. I think we should wait a bit for feedback from mypy folks.

You say that regarding the init/new issue or your original issue here?

If it is from your original issue, I insist that models.ForeignKey["Test"]("test.Test", models.CASCADE, null=True) is wrong, you should do models.ForeignKey[Optional["Test"]]("test.Test", models.CASCADE, null=True).

Think of that this way: If you write foo = ["a", "b", "c"], the infered type will be List[str]. If you write foo = ["a", "b", None] the infered type will be List[str | None]. If you type it with foo: List[str] = ["a", "b", None] you have an error. When you do models.ForeignKey[<T>], you are saying that that foreign key accepts values of type T. If T is not Optional[T] then the overload in __new__ expects null=False. More specifically, there's only one option for null=True and that's this overload, and the inferred type would be Optional[T], there's no other possibility.

If it is regarding __init__/__new__, I think it is better to discuss that in the other issue I opened to keep everything related in the same thread.

Yeah, let's talk about my issue with foreign keys here.

Im thinking of it this way, at each step i will attach reveal_type from pyright and mypy:

field = models.ForeignKey["Post"]
reveal_type(field)
# mypy: Revealed type is "def (to: Union[Type[tests.trout.models.Post*], builtins.str], ...
# pyright: Type of "field" is "Type[ForeignKey[Post]]"

At this state, before calling, it is Type[ForeignKey[Post]], this is correct.

Next, we call it:

field = models.ForeignKey["Post"]("Post", models.CASCADE, null=True)
reveal_type(field)
# mypy: Revealed type is "django.db.models.fields.related.ForeignKey[Union[tests.trout.models1.Post*, None]]"
# pyright: error: Argument of type "Literal[True]" cannot be assignedto parameter "null"

After calling, __new__ override returns correct optional type. Before calling it is not optional, after calling, it is. And this is correct.

The only thing that prevents me from doing this, is:

Pyright does not support inferring types from overrides in __init__, like (self: ForeignKey[M], ...
Mypy requires those overrides to be there, becouse it can not correctly infer types from __new__ when there is __init__.

If T is not Optional[T] then the overload in __new__ expects null=False. More specifically, there's only one option for null=True and that's this overload, and the inferred type would be Optional[T], there's no other possibility.

Actually, i think it works not exactly like this:

__new__ does not have types on self, so it would accept anything. And it returns ForeignKey[Optional[M]] or normal ForeignKey[M].

__init__ does have type restrictions on self, so it fails in pyright when we don't write Optional[Post]. Why it works in mypy, without Optional[Post] i don't know, but i think that's that undocumented backwards type inference, and not respecting types from __new__.

After calling, __new__ override returns correct optional type. Before calling it is not optional, after calling, it is. And this is correct.

The only thing that prevents me from doing this, is:

Pyright does not support inferring types from overrides in __init__, like (self: ForeignKey[M], ...

Mypy requires those overrides to be there, becouse it can not correctly infer types from __new__ when there is __init__.

Yes, pyright does not support inferring types from __init__ and I had a long discussion already there regarding that. And it also makes sense, it is __new__ that creates the object, __init__ just initializes it.

And IMO mypy is very wrong here. It is like if I did:

some_list: List[str] = ["a", "b", 123]

And it converts my user-typed List[str] to List[str | int]. It probably did that because of the init/new issues, which should be solved if we remove __init__ altogether.

Note that your example is not being typed inferred, you chose the type of your ForeignKey to be models.ForeignKey["Post"] and not models.ForeignKey[Optional["Post"]], and there's no way null=True is accepted for that case.

If T is not Optional[T] then the overload in __new__ expects null=False. More specifically, there's only one option for null=True and that's this overload, and the inferred type would be Optional[T], there's no other possibility.

Actually, i think it works not exactly like this:

__new__ does not have types on self, so it would accept anything. And it returns ForeignKey[Optional[M]] or normal ForeignKey[M].

__init__ does have type restrictions on self, so it fails in pyright when we don't write Optional[Post]. Why it works in mypy, without Optional[Post] i don't know, but i think that's that undocumented backwards type inference, and not respecting types from __new__.

I think there are 2 misunderstandings here:

1) As I said above, the overloads on __new__/__init__ will only infer the type when you do not type it by yourself. Think of it like in this example:

_T = TypeVar("_T")

def foo(arg: List[_T]) -> _T:
    ...

result = foo(["a", "b", "c"])  # result should be str
result = foo(["a", "b", "c", 123])  # result should be str | int

typed_list: List[str | int | None] = ["a", "b", "c"]
result = foo(typed_list)  # result should be str | list | None
result: str = foo(typed_list)  # this is an error because foo can return str | list | None, and result only expects str

2) __init__ can't be expected to infer self's type because it is not expected to do so. Who creates the object is __new__, and it controls the type of what it is creating, __init__ just initializes it. Both are specified today because of this long note that I wrote, which I'm now realizing that although it works, it is too complex and create issues like this.

I want to thank you for your patience, and for trying to explain all this.

I've read that note, and i think it is correct, and it is actually proper way type fields.

I understand your example with lists, but lists does not override __new__, and we do.

And you're typing list in your example, i don't type fields.

# This is not typing, field should be type inferred
field = models.ForeignKey[Post]()
# This is typing
field: models.ForeignKey[Post] = ...

Let's consider following example, without all complexity from django:

# related_field_minimal.pyi
from typing import Generic, Literal, Optional, TypeVar, overload

class Model:
    ...

_M = TypeVar("_M", bound=Optional[Model])

class ForeignKey(Generic[_M]):
    @overload
    def __new__(
        cls,
        null: Literal[False] = ...,
    ) -> ForeignKey[_M]: ...

    @overload
    def __new__(
        cls,
        null: Literal[True],
    ) -> ForeignKey[Optional[_M]]: ...

# models.py
from typing import Type
from related_field_minimal import ForeignKey, Model

class Post(Model):
    pass

def create_fk_field(post_class: Type[ForeignKey[Post]]):
    reveal_type(post_class)
    field = post_class(null=True)
    reveal_type(field)
    return field

class Comment:
    post = ForeignKey[Post](null=True)
    another_post = create_fk_field(ForeignKey[Post])

I've added function create_fk_field to illustrate what happens here.

We're not typing post, like this: post: ForeignKey[Post] = ..., we're typing class which would be called to create field.

Before we call it - it is Type[ForeignKey[Post]], after we call it - it becomes ForeignKey[Post | None], and both mypy and pyright are handling this correctly.

> mypy models.py
models.py:10: note: Revealed type is "Type[related_field_minimal.ForeignKey[models.Post]]"
models.py:12: note: Revealed type is "related_field_minimal.ForeignKey[Union[models.Post, None]]"

> pyright models.py
...
  /home/serg/src/tests/models.py:10:17 - information: Type of "post_class" is "Type[ForeignKey[Post]]"
  /home/serg/src/tests/models.py:12:17 - information: Type of "field" is "ForeignKey[Post | None]"

I want to thank you for your patience, and for trying to explain all this.

Np! Just knowing that I was helpful makes happy :)

I've read that note, and i think it is correct, and it is actually proper way type fields.

I understand your example with lists, but lists does not override __new__, and we do.

And you're typing list in your example, i don't type fields.

They don't because they are builtin. The only special thing about __new__ is that it dictates how an instance is going to be created. You can literally change the kind of the instance, which is not possible with init. For example:

class Foo:
    def __new__(cls, *args, **kwargs):
        return "somestring"

reveal_type(Foo())  # Type of "Foo()" is "Literal['somestring']"

You can't do the same on __init__ and that's the point of it not being able of changing the type. As you can see in my example above, I could instead of returning the string instantiate another Bar class for example and pass *args/**kwargs to it, which means that it would need to support those.

# This is not typing, field should be type inferred
field = models.ForeignKey[Post]()
# This is typing
field: models.ForeignKey[Post] = ...

Both are typing actually, and both types are very well defined, the only difference is that the second one is explicit from the variable perspective. Let me give you some examples.

# For example, I can do the same as you did with a list for example:
a = List[str](['a', 'b'])
reveal_type(a)  # Type of "a" is "List[str]"

# This is a type error the same way as if I did `a: List[str]`,
# it will not make `a` become `List[str | int]` all of the sudden
a.append(1) 

# But I can change `a` to something else and it will accept,
# because I'm not modifying `a` here, just pointing it somewhere else
a = [1, 2, 3]
reveal_type(a)

# Now this is an error because you are saying that the variable
# `b` will always be `List[str]` and can't change reference to something else
b: List[str] = ["a", "b"]
b = [1, 2, 3]

The same happens on your code for fields, the only difference is that the generic value, if not given, is "captured" by the function that you've called.

Here is maybe a more specific example that will give you the same error:

from typing import Generic, TypeVar

_T = TypeVar("_T")

class Foo(Generic[_T]):
    def __init__(self, v: list[_T]):
        ...

    def some_method(self) -> _T:
        ...

foo1 = Foo([1, 2, 3])
reveal_type(foo1)
reveal_type(foo1.some_method())
foo2 = Foo[int]([1, 2, 3])
reveal_type(foo2)
reveal_type(foo2.some_method())
foo3: Foo[int] = Foo([1, 2, 3])
reveal_type(foo3)
reveal_type(foo3.some_method())

foo1 = Foo([1, 2, 3, "acb"])
reveal_type(foo1)
reveal_type(foo1.some_method())
foo2 = Foo[int]([1, 2, 3, "abc"])
reveal_type(foo2)
reveal_type(foo2.some_method())
foo3: Foo[int] = Foo([1, 2, 3, "abc"])
reveal_type(foo3)
reveal_type(foo3.some_method())

The output is:

test.py
  test.py:15:13 - information: Type of "foo1" is "Foo[int]"
  test.py:17:13 - information: Type of "foo2" is "Foo[int]"
  test.py:19:13 - information: Type of "foo3" is "Foo[int]"
  test.py:22:13 - information: Type of "foo1" is "Foo[int | str]"
  test.py:23:17 - error: Argument of type "list[int | str]" cannot be assigned to parameter "v" of type "list[int]" in function "__init__"
    TypeVar "_T@list" is invariant
      Type "int | str" cannot be assigned to type "int"
        "str" is incompatible with "int" (reportGeneralTypeIssues)
  test.py:24:13 - information: Type of "foo2" is "Foo[int]"
  test.py:25:18 - error: Expression of type "Foo[int | str]" cannot be assigned to declared type "Foo[int]"
    TypeVar "_T@Foo" is invariant
      Type "int | str" cannot be assigned to type "int"
        "str" is incompatible with "int" (reportGeneralTypeIssues)
  test.py:26:13 - information: Type of "foo3" is "Foo[int]"
2 errors, 0 warnings, 6 informations

As you can see, for a Foo[int], be it typed foo: Foo[int] or foo = Foo[int](...) there's no overload available that accepts an input that contains a str. And when you let the

And I know, you might be thinking "but then init can type the object" and the answer is "mostly". Just like with any function call, if the arguments are mapped to a TypeVar, it will automatically give context to it as a Union of all the possibilities. But in that example, how would you add a null: bool to its __init__ and capture it as an Optional modified to _T? It is just not possible without typing self, which pyright doesn't support anymore (and mypy shouldn't either).

The way you do is something like this:

class Foo(Generic[_T]):
    def __init__(self, foo: list[_T], bar: str = ..., null: bool = ...):
        ...

    @overload
    def __new__(
        cls,
        foo: list[_T],
        *args,
        null: Literal[True] = ...,
        **kwargs,
    ) -> "Foo[_T | None]":
        ...

    @overload
    def __new__(
        cls,
        foo: list[_T],
        *args,
        null: Literal[False] = ...,
        **kwargs,
    ) -> "Foo[_T]":
        ...

class Bar(Foo[_T]):
    ...

__new__ can capture both and use that overload to define Foo. This also works flawless on pyright, where subclasses will have the subclass type instead of Foo type, and wen can capture *args/**kwargs of the stuff we don't need for typing on __new__ and still have it validated with __init__, which only needs to be defined once.

The problem here lies in mypy. With that exact same code, because Foo has both __init__ and __new__, mypy will get confused. More specifically: 1) If you create a Bar(Foo[_T]) subclass and instantiate it, pyright will say it is Bar, mypy will say it is Foo. 2) It will not type Foo[int]/Foo[int | None] based on the presence of null because it is rellying totally on __init__ now.

The fix? Well, the proper fix would be on mypy's side. Another one would be to add 2 __init__ overloads on Foo, which means that we would need to add __new__ overloads now on Bar, etc.

But remove the __init__ from it. You still have to write __new__ twice, with all the accepted arguments, but now mypy works exactly the same way as pyright works, no surprises.

So, we gonna remove __init__?

I think this is best we can do until mypy fixes it's problems.

I could do this for ForignKeys, or you want to do this this yourself?

@last-partizan IMO it is best to remove __init__ and keep only __new__. That's the reason I opened that other issue, because its not an easy task and its a totally different story having to write 2 or 4 __new__ overloads with 4 combinations parameters.

I would like to hear @sbdchd 's opinion before wasting some time in it =P. Although, as I mentioned above, it is mostly mechanical work, the code will get a lot simpler.

And unfortunately you can't just do ForeignKeys. As long as at least one parent has __init__ on them, __new__ will not have any effect for mypy, meaning that the __init__ from Field should be removed and all the __new__ need to be changed at the same time...

Sounds good to me as long as Pyright and mypy both work

sbdchd / django-types

False positive for argument mismatch #85