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

Embedded models have to be defined before any normal models that reference them #400

Open
duhby opened this issue Jan 5, 2024 · 4 comments
Labels
bug Something isn't working

Comments

@duhby
Copy link

duhby commented Jan 5, 2024

Bug

No matter how you type hint a field in the base model that references an embedded one (including future annotations), defining indexes for fields that are in the embedded model, or trying to query/sort by fields that are in the embedded model throws an error when the embedded model is defined after the base model, but not when it is defined before.

Current Behavior

from odmantic import AIOEngine, EmbeddedModel, Index, Model
from odmantic.query import asc


class Bar(Model):
    bar: "Foo"

    model_config = {
        "indexes": lambda: [Index(asc(Bar.bar.foo))]
    }


class Foo(EmbeddedModel):
    foo: int


engine = AIOEngine()
await engine.configure_database([Bar])  # throws error
await engine.find(Bar, sort=asc(Bar.bar.foo))  # also throws error

The error that gets thrown both times is: AttributeError: operator foo not allowed for ODMField fields

However, when you switch the model definitions around like this:

...

class Foo(EmbeddedModel):
    foo: int


class Bar(Model):
    bar: "Foo"

    model_config = {
        "indexes": lambda: [Index(asc(Bar.bar.foo))]
    }

...

it behaves as expected (see "Expected behavior").

Expected behavior

No errors are thrown and the engine methods work as intended.

If it's intentional or expected behavior, it should be clear in the documentation.

Environment

  • ODMantic version: 1.0.0
  • MongoDB version: ...
  • Pydantic version: 2.5.3
  • Pydantic-core version: 2.14.6
  • Python version: 3.12.0

Additional context

It doesn't matter if you use foo: int = Field(...) or just have it as foo: int. The behavior is unchanged.
It doesn't matter if you use bar: "Foo", bar: Foo, or bar: Foo # (with future annotations). The behavior is unchanged.
This is also the case for reference fields.

@duhby duhby added the bug Something isn't working label Jan 5, 2024
@rollebolle123
Copy link

But, that makes sense? :O The embedded model should be defined first so a definition exists?

Also:
class Bar(Model):
bar: "Foo" <------ should lose the quotation marks?

aaaand
[Index(asc(Bar.bar.foo)] <--- missing a parentheses :D :
[Index(asc(Bar.bar.foo))]

@duhby
Copy link
Author

duhby commented Jan 8, 2024

Well as I mentioned, it doesn't matter if the quotations are there or not for both cases, and I didn't test the example code specifically so I missed the parentheses. I just think it should be specified in the documentation because it's not obvious that it should be defined before (other odm libraries don't have this behavior).

When you say "the embedded model should be defined first so a definition exists," that should (intuitively) only be necessary for the type hint if you want to define it without quotations. The reason I say it's not intuitive or obvious is because by the time any code is run regarding creating indexes or creating search queries, both of the models have already been defined.

I'm not saying the behavior is bad, it just needs to be clarified somewhere, and it might be unintended.

@duhby
Copy link
Author

duhby commented Jan 8, 2024

Also, what's with the unnecessary condescension? You clearly didn't take the time to fully understand the original post.

@rollebolle123
Copy link

No, sorry, I am not trying to be condescending.
And maybe I did not understand the original problem. Aaaaand, I had no idea you could put quotes around types to forward declare them. Then it makes sense to me :D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants