pyrsistent icon indicating copy to clipboard operation
pyrsistent copied to clipboard

PRecord field types in Python 3.7

Open agberk opened this issue 5 years ago • 4 comments

Consider the following code snippet contained in a python module named pyrsistent_test.py:

from typing import Mapping

from pyrsistent import field, PRecord

class MyPRecord(PRecord):
  f = field(type=Mapping)


def main():
  p = MyPRecord(f={})

  print(p)


if __name__ == '__main__':
  main()

When running this under Python 3.6 the output is as follows:

(python-3.6) $ python pyrsistent_test.py 
MyPRecord(f={})
(python-3.6) $

When running this under Python 3.7 the output is as follows:

(python-3.7) $ python pyrsistent_test.py 
Traceback (most recent call last):
  File "pyrsistent_test.py", line 5, in <module>
    class MyPRecord(PRecord):
  File "pyrsistent_test.py", line 6, in MyPRecord
    f = field(type=Mapping)
  File "/home/aaron/workspace/pyrsistent/pyrsistent/_field_common.py", line 120, in field
    types = set(maybe_parse_user_type(type))
  File "/home/aaron/workspace/pyrsistent/pyrsistent/_checked_types.py", line 88, in maybe_parse_user_type
    'Type specifications must be types or strings. Input: {}'.format(t)
TypeError: Type specifications must be types or strings. Input: typing.Mapping
(python-3.7) $

I've been using the typing module for defining most of my PRecord field types but have only begun to think about upgrading to Python 3.7 hence running into this problem now.

Is it not expected to use things from the typing module when defining the types for a PRecord field - not sure if I've missed another way of doing things?

I do notice that the type of things in the typing module has changed between 3.6 and 3.7:

Python 3.6.9 (default, Jul  3 2019, 15:36:16) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import typing
>>> type(typing.Mapping)
<class 'typing.GenericMeta'>
>>>
Python 3.7.5 (default, Oct 15 2019, 21:38:37) 
[GCC 5.4.0 20160609] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import typing
>>> type(typing.Mapping)
<class 'typing._GenericAlias'>
>>>

I did have a go at adding another elif state to maybe_parse_user_type in https://github.com/tobgu/pyrsistent/blob/master/pyrsistent/_checked_types.py#L62-L89 which solved the issue though this doesn't really seem like the right thing to do and I haven't fully comprehended the implications of changing this function!:

elif isinstance(t, typing._VariadicGenericAlias) or isinstance(t, typing._GenericAlias):
        return [t.__origin__]

Happy to do a PR but feel I would need some guidance first.

agberk avatar Oct 31 '19 20:10 agberk

Thanks for reporting this. I don't think the initial thought was to put types from the types module in there but just concrete or abstract, "normal", types. Most of this was written before typing was getting momentum in Python though. Today I definitely think it's a valid use case that should be supported!

I will have to look more into the inner structure of the types package to be able to provide any guidance on how to solve it. The current suggestion seems fine except for accessing what seems to be meant as private types of the types package.

tobgu avatar Oct 31 '19 21:10 tobgu

I know it's been a while, did you ever have a chance to look at this more? I'm still interested in resolving this if you have any further thoughts on it.

agberk avatar Dec 15 '20 01:12 agberk

Hello, I'm also considering Pyrsistent for my project, but not being able to use pmap_fields with Any, AnyStr or other NewTypes could be a dealbreaker... Any thought on how to get around this?

Gesprye avatar Feb 18 '21 19:02 Gesprye

Although I mentioned at the time I hadn't fully comprehended the implications of my change, looking into things a bit more using __origin__ won't work for things like typing.Any; I'm not sure if there's a succinct solution that would slot into maybe_parse_user_type to handle everything in the typing package.

As for "workarounds" - in my case collections.abc.Mapping can be used instead of typing.Mapping. From python3.9 typing.Mapping is actually deprecated in favour of collections.abc.Mapping which also now supports [] (e.g. collections.abc.Mapping[str, int] so a change from

from typing import Mapping

to

from collections.abc import Mapping

Works across a codebase.

As for @Gesprye use case - I believe if you define a CheckedPMap but leave out defining either __key_type__ or __value_type__ (whichever you want to allow to be any type) should achieve what you want for Any; for AnyStr I'm guessing you could use (str, bytes) as the tuple of types? Not sure what you could do about a NewType.

I guess as a general point, a field allows no types to be passed (denoting Any?) whereas in pmap_field and pvector_field it's a required argument (although as mentioned above it seems like you can get around this defining a CheckedPMap or CheckedPVector leaving out the types).

agberk avatar Sep 09 '21 09:09 agberk