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

Type Unions Broken #591

Closed
hthall13 opened this issue Feb 2, 2024 · 1 comment · Fixed by #603
Closed

Type Unions Broken #591

hthall13 opened this issue Feb 2, 2024 · 1 comment · Fixed by #603

Comments

@hthall13
Copy link

hthall13 commented Feb 2, 2024

HashModels don't appear to support pydantic Unions.

# test.py

from typing import Union
from redis_om import JsonModel, HashModel

class JModel(JsonModel):
    param: Union[str, int]

class HModel(HashModel):
    param: Union[str, int]

The issue prevents initialization of the class:

$ python3 test.py 
Traceback (most recent call last):
  File "/home/user/project/test.py", line 7, in <module>
    class HModel(HashModel):
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1195, in __new__
    new_class = super().__new__(cls, name, bases, attrs, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/pydantic/v1/main.py", line 282, in __new__
    cls = super().__new__(mcs, name, bases, new_namespace, **kwargs)
  File "/usr/lib/python3.10/abc.py", line 106, in __new__
    cls = super().__new__(mcls, name, bases, namespace, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1496, in __init_subclass__
    if issubclass(origin, typ):
  File "/usr/lib/python3.10/typing.py", line 1158, in __subclasscheck__
    return issubclass(cls, self.__origin__)
TypeError: issubclass() arg 1 must be a class

I get the same error when incorporating Unions in JsonModel Fields as well.

# test2.py

from typing import Union
from redis_om import JsonModel, Field

class JModel(JsonModel):
    param: Union[str, int] = Field(default = 0)

Produces:

$ python3 test2.py 
Traceback (most recent call last):
  File "/home/user/project/test2.py", line 4, in <module>
    class JModel(JsonModel):
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1195, in __new__
    new_class = super().__new__(cls, name, bases, attrs, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/pydantic/v1/main.py", line 282, in __new__
    cls = super().__new__(mcs, name, bases, new_namespace, **kwargs)
  File "/usr/lib/python3.10/abc.py", line 106, in __new__
    cls = super().__new__(mcls, name, bases, namespace, **kwargs)
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1675, in __init_subclass__
    cls.redisearch_schema()
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1742, in redisearch_schema
    schema_parts = [schema_prefix] + cls.schema_for_fields()
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1753, in schema_for_fields
    cls.schema_for_type(json_path, name, "", _type, field.field_info)
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1890, in schema_for_type
    elif any(issubclass(typ, t) for t in NUMERIC_TYPES):
  File "/home/user/.local/lib/python3.10/site-packages/redis_om/model/model.py", line 1890, in <genexpr>
    elif any(issubclass(typ, t) for t in NUMERIC_TYPES):
TypeError: issubclass() arg 1 must be a class

However excluding the Field class seems to keep this from happening:

# working code

from typing import Union
from redis_om import JsonModel

class JModel(JsonModel):
    param: Union[str, int]

if __name__ == "__main__":
    model = JModel(param=0)

Is this related to some problem with later pydantic versions? It looks like there are at least 3 PRs waiting to be reviewed to solve that problem: #548, #556, #577

redis-om: 0.2.1
pydantic: 2.0
pydantic_core = 2.0.1

slorello89 added a commit that referenced this issue Apr 5, 2024
@slorello89
Copy link
Member

@hthall13 this will be somewhat relieved by #603 with a couple of caveats:

  1. Union's aren't really appropriate for indexing in Redis (it really needs to know the type ahead of time to make an intelligent choice as to how to index it)
  2. If you are using a hash model, Redis OM will not be able to properly rematerialize the int type (as it gets a string and it see's that a string is a valid type)

I've added some tests to #603 to test this case.

slorello89 added a commit that referenced this issue May 2, 2024
…603)

* refactoring model to use pydantic 2.0 types and validators

* adding tests for #591

* adding tests with uuid

* readme fixes

* fixing typo in NOT_IN
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

Successfully merging a pull request may close this issue.

2 participants