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

PRecord field types in Python 3.7 #181

Open
agberk opened this issue Oct 31, 2019 · 4 comments
Open

PRecord field types in Python 3.7 #181

agberk opened this issue Oct 31, 2019 · 4 comments

Comments

@agberk
Copy link

agberk commented Oct 31, 2019

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.

@tobgu
Copy link
Owner

tobgu commented Oct 31, 2019

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.

@agberk
Copy link
Author

agberk commented Dec 15, 2020

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.

@Gesprye
Copy link

Gesprye commented Feb 18, 2021

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?

@agberk
Copy link
Author

agberk commented Sep 9, 2021

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).

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

3 participants