tobgu / pyrsistent

Persistent/Immutable/Functional data structures for Python
MIT License
2.03k stars 147 forks source link

Type checking tuple type for fields causes infinite recursion #291

Open cromachina opened 3 weeks ago

cromachina commented 3 weeks ago

Specifying a tuple type for a field seems to break pyrsistent's type checker.

Pyrsistent version 0.20.0 Python 3.12 Linux (NixOS)

Minimal example test.py:

from pyrsistent import *

IVec2 = tuple[int, int]

class Mask(PClass):
    position = field(type=IVec2)

Load the module:

python -m test.py

Output:

Traceback (most recent call last):
  File "<frozen runpy>", line 189, in _run_module_as_main
  File "<frozen runpy>", line 112, in _get_module_details
  File "/home/cro/aux/Projects/crowpainter/src/crowpainter/test.py", line 5, in <module>
    class Mask(PClass):
  File "/home/cro/aux/Projects/crowpainter/src/crowpainter/test.py", line 6, in Mask
    position = field(type=IVec2)
               ^^^^^^^^^^^^^^^^^
  File "/nix/store/kq39qlgbmj8qib3jc600d1k0pfk1nw3d-python3.12-pyrsistent-0.20.0/lib/python3.12/site-packages/pyrsistent/_field_common.py", line 121, in field
    types = set(maybe_parse_user_type(type))
                ^^^^^^^^^^^^^^^^^^^^^^^^^^^
...
...
  File "/nix/store/kq39qlgbmj8qib3jc600d1k0pfk1nw3d-python3.12-pyrsistent-0.20.0/lib/python3.12/site-packages/pyrsistent/_checked_types.py", line 92, in maybe_parse_user_type
    return tuple(e for t in ts for e in maybe_parse_user_type(t))
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/kq39qlgbmj8qib3jc600d1k0pfk1nw3d-python3.12-pyrsistent-0.20.0/lib/python3.12/site-packages/pyrsistent/_checked_types.py", line 92, in <genexpr>
    return tuple(e for t in ts for e in maybe_parse_user_type(t))
                                        ^^^^^^^^^^^^^^^^^^^^^^^^
  File "/nix/store/kq39qlgbmj8qib3jc600d1k0pfk1nw3d-python3.12-pyrsistent-0.20.0/lib/python3.12/site-packages/pyrsistent/_checked_types.py", line 81, in maybe_parse_user_type
    is_iterable = isinstance(t, Iterable)
                  ^^^^^^^^^^^^^^^^^^^^^^^
RecursionError: maximum recursion depth exceeded
cromachina commented 3 weeks ago

So I found that the problem is that the type checking is simply not complete, and I think if you want complete type checking in pyrsistent, then you should probably use some other type checking library, as python's builtin type checking seems inadequate for this. For example, even if you add separate instance checks for types.GenericAlias or types.UnionType, the builtin functions isinstance and issubclass will fail if the checked object is types.GenericAlias: https://docs.python.org/3/library/stdtypes.html#types-genericalias

The builtin functions isinstance() and issubclass() do not accept GenericAlias types for their second argument:

So you have to create a bunch of cases to handle all of these type-like objects which are not actually types.

However, I think the solution should be to allow types to be specified how the user wants, with the declarations that python already allows (like using a union str | int instead of a list [str, int] to specify multiple types) and then have pyrsistent do not do type checking at all of its own. That way we can still benefit from having things like an IDE working nicely (which is what I think most people use type annotations in python for) and use other tools like mypy if we really want to make sure all of the types unify statically.

If for some reason the type checking must remain, then I think it should be able to be disabled by the user (because it's incomplete and produces errors like above).

cromachina commented 3 weeks ago

Similar type checking woes mentioned here: https://github.com/tobgu/pyrsistent/issues/181

cromachina commented 3 weeks ago

Seems like I can bypass the library's type checking by putting my type right on the variable instead, so that solves my particular issue.

from pyrsistent import *

tuple_type = tuple[int, int]

class Test(PClass):
    x:tuple_type = field()