BeanieODM / beanie

Asynchronous Python ODM for MongoDB
http://beanie-odm.dev/
Apache License 2.0
1.99k stars 211 forks source link

[BUG] Mypy:: Type error: base class "Document" defined the type as "Optional[PydanticObjectId]" #336

Closed amoallim15 closed 1 year ago

amoallim15 commented 2 years ago

Describe the bug (error: Incompatible types in assignment (expression has type "UUID", base class "Document" defined the type as "Optional[PydanticObjectId]"))

To Reproduce

# Please add a code snippet here, that reproduces the problem completely
from beanie import Document
from uuid import uuid4, UUID

class MembershipDao(Document):
    id: UUID = Field(default_factory=uuid4)
    name: str

Expected behavior no errors. temporary solution:

from beanie import Document
from uuid import uuid4, UUID

class MembershipDao(Document):
    id: UUID = Field(default_factory=uuid4)  # type: ignore
    name: str

Additional context N/A

buckldav commented 2 years ago

The Document class already has a built in id field that is generated when the document is created. It's type is PydanticObjectId, which clashes with the UUID type that you have. I'd recommend simply getting rid of your own id field and use the built-in. Docs: https://roman-right.github.io/beanie/api-documentation/document/

Mark90 commented 1 year ago

The Document class already has a built in id field that is generated when the document is created. It's type is PydanticObjectId, which clashes with the UUID type that you have. I'd recommend simply getting rid of your own id field and use the built-in. Docs: https://roman-right.github.io/beanie/api-documentation/document/

But the docs also mention you can use UUID as id type: https://beanie-odm.dev/tutorial/async/defining-a-document/#id

So maybe the id: Optional[PydanticObjectId] = None should be typed such that it allows UUID as well

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity.

Mark90 commented 1 year ago

The Document class already has a built in id field that is generated when the document is created. It's type is PydanticObjectId, which clashes with the UUID type that you have. I'd recommend simply getting rid of your own id field and use the built-in. Docs: https://roman-right.github.io/beanie/api-documentation/document/

But the docs also mention you can use UUID as id type: https://beanie-odm.dev/tutorial/async/defining-a-document/#id

So maybe the id: Optional[PydanticObjectId] = None should be typed such that it allows UUID as well

More concretely, this would mean changing

id: Optional[PydanticObjectId] = None

to

id: Union[PydanticObjectId, UUID, None] = None

(+ of course making all the other code ready for this, writing tests, making mypy happy)

Did give this a quick try the other day, it's harder than it looks; so before me (or someone else) take a stab at a PR, it would help knowing if the maintainers agree on the approach :)

github-actions[bot] commented 1 year ago

This issue is stale because it has been open 30 days with no activity.

github-actions[bot] commented 1 year ago

This issue was closed because it has been stalled for 14 days with no activity.