Skip to content
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

List[int] is incompatible with List[Union[float]] #67

Closed
jolaf opened this issue Aug 26, 2019 · 9 comments
Closed

List[int] is incompatible with List[Union[float]] #67

jolaf opened this issue Aug 26, 2019 · 9 comments

Comments

@jolaf
Copy link

jolaf commented Aug 26, 2019

The following code:

from typing import cast, List

from pytypes import TypeChecker

def f() -> List[float]:
    return cast(List[float], [2])

with TypeChecker():
    f()
    print("OK")

fails as follows:

$ python3 Test.py
Traceback (most recent call last):
  File "Test.py", line 9, in <module>
    f()
  File "Test.py", line 6, in f
    return cast(List[float], [2])
pytypes.exceptions.ReturnTypeError: 
  __main__.f
  returned incompatible type:
Expected: List[float]
Received: List[int]
@Stewori
Copy link
Owner

Stewori commented Aug 26, 2019

Is this really about Union? I suspect it would behave the same with

def f() -> List[float]:.

That said, this behavior is correct in the sense that List is invariant rather than covariant.
I admit there is kind of flaw in concluding that [2] is a List[int] and not maybe a List[float] or List[Union[int, whatever]]. That issue is also discussed at #21. However I admit the discussion there drifted off from a distinct original issue.
Still this requires more thought and isn't an easy fix. So for now I just repeat the recommendation to use Sequence rather than List for such cases.

@jolaf
Copy link
Author

jolaf commented Aug 27, 2019

Sorry, it's really not about Union, I've fixed the test.

@jolaf
Copy link
Author

jolaf commented Aug 27, 2019

As far as I understand, the concepts of invariance/covariance are mostly actual when doing static type checking, when we have no information on actual variable contents, and can only talk about what is expected and what is acceptable.

When doing a runtime check, the static typing information is not fully relevant, as it's not really used by runtime. In runtime, there's no difference between List[int], List[float], List[Any], List, list() and [].

Moreover, there's no way to specify which of those is meant. For example, we can annotate return value (see the test in this issue), like return cast(List[float], [2]), but that would not actually change anything in runtime.

So, in runtime, [2] is a perfectly valid List[float] as it can do anything List[float] can do and can't do anything List[float] can't do, so the warning we see in this issue is not really useful for anything, so I treat it as false positive.

Though static type annotations are very useful for runtime type checking, it's clear to me that runtime type checking can NOT be performed exactly like static type checking is performed, it's a different matter and should use different logic.

@jolaf
Copy link
Author

jolaf commented Aug 27, 2019

I understand the suggestion to use Sequence instead of List wherever possible, but sometimes a mutable sequence is really necessary.

Also, I see the main purpose of tools like pytypes in looking for obscure bugs in exisiting code bases, and false positives like this really don't help when they happen on perfectly valid code fragments.

@Stewori
Copy link
Owner

Stewori commented Aug 27, 2019

I am going to open a separate issue about the List-invariance and solution ideas. However don't expect a solution there soon. You can try the Sequence workaround, but I admit that is mainly preferable for argument types rather than return types.

Regarding static vs runtime typechecking, maybe I can add some context:
pytypes is mainly intended as a typing toolbox and the capability of runtime typechecking is more a usecase. My main goal was to provide consistent functions for subtype checking (is_subtype) and determining the type of a given object in PEP 484 terms (deep_type). The runtime typechecking works by combining these functionalities. This design is not going to change as it would mean to rewrite pytypes I guess. So there needs to be a solution that fits into this design. I think that's possible, but as I told you don't expect it to happen soon as it is rather involved work.

That said, I noticed that you are also active in typeguard. If you just need pytypes to enable stubfile support (Note: overload is currently not supported which rules out many stubfiles from typeshed) you may want to try using @annotations to pull in type info from stubfiles or comments and use another typechecker on top for now. (Never tried this, don't know whether it works.)

@jolaf
Copy link
Author

jolaf commented Aug 27, 2019

That's pretty understandable, for sure.

Yes, I came from typeguard to pytypes looking for a tool that can also check the correctness of API and 3rd-party library calls.

However I don't exactly understand how @annotations would help me do it. My project uses quite a lot of standart and 3rd-party libraries, some of them are annotated with typeshed stubs, for others I have created annotation stubs myself. How do I use @annotations to help a tool like typeguard to use those stubs?

@jolaf
Copy link
Author

jolaf commented Aug 29, 2019

See #75.

@Stewori
Copy link
Owner

Stewori commented Aug 29, 2019

@annotations populates the __annotations__ property of the callable from type comments or stubfiles. __annotations__ is usually populated by the python interpreter from the usual type annotations. Libraries check for it to retrieve type information for whatever purpose. I think also typing.get_type_hints takes its info from __annotations__ but the typing module changed frequently and I havent checked for a while if that's still the case.
Depending on how typeguard operates it may or may not help to populate __annotations__ previous to the typechecker running over it.
Will check #75 as I find time.

@Stewori
Copy link
Owner

Stewori commented Aug 29, 2019

Closing this in favor of #76.

@Stewori Stewori closed this as completed Aug 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants