Skip to content

handle default issue with multiple inheritance #687

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

Merged
merged 3 commits into from
Apr 4, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions aredis_om/model/model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1320,11 +1320,13 @@ def __new__(cls, name, bases, attrs, **kwargs): # noqa C901
meta = meta or getattr(new_class, "Meta", None)
base_meta = getattr(new_class, "_meta", None)

if len(bases) == 1:
for f_name in bases[0].model_fields:
field = bases[0].model_fields[f_name]
print(field)
new_class.model_fields[f_name] = field
if len(bases) >= 1:
for base_index in range(len(bases)):
model_fields = getattr(bases[base_index], "model_fields", [])
for f_name in model_fields:
field = model_fields[f_name]
print(field)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would you mind deleting this extra print while you are in here?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to push a commit removing it to your branch but my push was rejected.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

new_class.model_fields[f_name] = field

if meta and meta != DefaultMeta and meta != base_meta:
new_class.Meta = meta
Expand Down
37 changes: 37 additions & 0 deletions tests/test_hash_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -976,3 +976,40 @@ class Child(Model):

assert rematerialized.age == 18
assert rematerialized.bio is None

@py_test_mark_asyncio
async def test_grandchild_class_expression_proxy():
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests pass on the current state of main, I see what you are getting at though, you are talking about multiple inheritance - so if you try to run this test on main:

@py_test_mark_asyncio
async def test_multiple_inheritance_class_expression_proxy():
    class Model(HashModel):
        first_name: str
        last_name: str
        age: int = Field(default=18)
        bio: Optional[str] = Field(default=None)

    class Sibling():
        is_sibling: bool = Field(default=True)

    class Child(Model, Sibling):
        is_child: bool = True

    await Migrator().run()
    m = Child(first_name="Steve", last_name="Lorello")
    assert m.age == 18
    await m.save()

    assert m.age == 18
    assert m.is_sibling
    assert m.is_child

    rematerialized = await Child.find(Child.pk == m.pk).first()

    assert rematerialized.age == 18
    assert rematerialized.age != 19
    assert rematerialized.bio is None
    assert rematerialized.is_sibling
    assert rematerialized.is_child

it will fail

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that is exactly it. I updated the test to reflect that. I wasn't totally understanding what the issue was on our side but now that I do, we can definitely fix it in our implementation. Either way, wanted to leave this up in case it is helpful.

# https://github.com/redis/redis-om-python/issues/669 seeing weird issue with child classes initalizing all their undefined members as ExpressionProxies
class Model(HashModel):
first_name: str
last_name: str
age: int = Field(default=18)
bio: Optional[str] = Field(default=None)

class Child(Model):
other_name: str = "test"

class GrandChild(Child):
grand_name: str = "test"

class GreatGrandChild(GrandChild):
great_name: str = "test"

await Migrator().run()
m = GreatGrandChild(first_name="Steve", last_name="Lorello")
assert m.age == 18
await m.save()

assert m.age == 18
assert m.other_name == "test"
assert m.grand_name == "test"
assert m.great_name == "test"

rematerialized = await GreatGrandChild.find(GreatGrandChild.pk == m.pk).first()

assert rematerialized.age == 18
assert rematerialized.age != 19
assert rematerialized.bio is None
assert rematerialized.other_name == "test"
assert rematerialized.grand_name == "test"
assert rematerialized.great_name == "test"
37 changes: 37 additions & 0 deletions tests/test_json_model.py
Original file line number Diff line number Diff line change
Expand Up @@ -1195,6 +1195,43 @@ class Child(Model):
assert rematerialized.age != 19
assert rematerialized.bio is None

@py_test_mark_asyncio
async def test_grandchild_class_expression_proxy():
# https://github.com/redis/redis-om-python/issues/669 seeing weird issue with child classes initalizing all their undefined members as ExpressionProxies
class Model(JsonModel):
first_name: str
last_name: str
age: int = Field(default=18)
bio: Optional[str] = Field(default=None)

class Child(Model):
is_new: bool = True

class GrandChild(Child):
is_newer: bool = True

class GreatGrandChild(GrandChild):
is_great: bool = True

await Migrator().run()
m = GreatGrandChild(first_name="Steve", last_name="Lorello")
assert m.age == 18
await m.save()

assert m.age == 18
assert m.is_new is True
assert m.is_newer is True
assert m.is_great is True

rematerialized = await GreatGrandChild.find(GreatGrandChild.pk == m.pk).first()

assert rematerialized.age == 18
assert rematerialized.age != 19
assert rematerialized.bio is None
assert rematerialized.is_new is True
assert rematerialized.is_newer is True
assert rematerialized.is_great is True

@py_test_mark_asyncio
async def test_merged_model_error():
class Player(EmbeddedJsonModel):
Expand Down