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

pytypes is_of_type: counter-intuitive result on semi-primitive types like list and dict #101

Open
Martmists-GH opened this issue Dec 10, 2020 · 5 comments

Comments

@Martmists-GH
Copy link

In [2]: from pytypes import is_of_type
In [3]: is_of_type([1, 2, 3], list)
Out[3]: False
In [4]: is_of_type({"a": True}, dict)
Out[4]: False

Python 3.9.0
PyTypes 1.0b5

@Stewori
Copy link
Owner

Stewori commented Dec 10, 2020

I think the rationale is that list behaves like List[Any] and List is invariant (because it's writable), see https://www.python.org/dev/peps/pep-0484/#covariance-and-contravariance. Also Dict is invariant in the key type, so that is in theory okay. I admit it is unintuitive and people frequently complain about List being invariant. However, that's more of an PEP 484 issue then. Note that there is this option pytypes.covariant_Mapping that tells pyytpes to treat the key type of mappings and dicts as covariant rather than invariant. However, I never found time and capacity to add something similar for list. I keep telling people to use Sequence[Any] instead because that's covariant.

That said, note that Python 3.9 is not supported, honestly so far I did not even check it out. Python 3.7 and 3.8 are almost fully supported on current master version, however not in 1.0b5. The latest release just supports Python 3.6.

@Stewori Stewori changed the title pytypes is_of_type fails on semi-primitive types like list and dict pytypes is_of_type: counter-intuitive result on semi-primitive types like list and dict Dec 10, 2020
@Martmists-GH
Copy link
Author

I keep telling people to use Sequence[Any] instead because that's covariant.

The issue here is that the types are supplied by users of my library, and having list and dict not be supported as types may cause issues. Would you recommend I do a hard-replace of list to List[Any] and dict to Dict[Any, Any], or is there a better solution?

@Stewori
Copy link
Owner

Stewori commented Dec 11, 2020

having list and dict not be supported

I claim they are supported, and what you observe is correct behavior. List[int] is not a subtype of List[Any] because it does not permit to add e.g. a str. Why would it serve your users to prefer incorrect behavior? Maybe the best way is to educate by raising a warning.

Would you recommend I do a hard-replace of list to List[Any]

Changing list to List[Any] wouldn't make a difference. If I understand correctly, your users do not understand the implications of an invariant type and you want to guess what they actually mean. It depends on the use case, but as noted above, when people specify list, Sequence[Any] is usually what they actually want. So if at all, better change list to Sequence[Any], that should solve it in most cases; still consider to raise a warning to inform users about the implicit change. (Btw, note that is_of_type((1, 2, 3), tuple) works as expected.)

For the Dict issue, simply use current master rather than the latest release. It has pytypes.covariant_Mapping == True by default. I cannot reproduce the issue concerning dict with current master:
is_of_type({"a": True}, dict) # True
Current master is anyway the better choice because of better Python 3.7, 3.8 support. Sorry I never found the time to polish it into release state. There is still a test failing that is non-trivial to fix and I cannot currently work on pytypes.

@Stewori
Copy link
Owner

Stewori commented Dec 11, 2020

In case you don't already know this trick: You can install current master via pip like explained here. It should also be possible to specify it as a dependency this way.

@Stewori
Copy link
Owner

Stewori commented Dec 14, 2020

It occurred to me that this is much the same issue as described in #76. The actual issue is that [1, 2, 3] is considered List[int] although it might rather be List[Union[int, whatever]]. In general, inferring the type of an (invariant) container from a limited sample is problematic and needs to be rethought like the solution idea described in #76. I should have pointed this out right from the beginning, however I had somewhat forgot about #76. That said, I cannot tell when I will be able to implement the solution described in #76. So you are still best off with a workaround for now (like changing list and List to Sequence[Any] or so).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants