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
smart_deepcopy causes sqlalchemy boolean value error on import #4184
Comments
Off the top of my head, I don't know. That's a config setting called |
from uuid import UUID, uuid4
import sqlalchemy as sa
import sqlalchemy.dialects.postgresql as pg
from pydantic import BaseModel as _BaseModel
from sqlalchemy.ext.declarative import declarative_base
class CommonBase(_BaseModel):
id: UUID = sa.Column(pg.UUID(as_uuid=True), primary_key=True, default=uuid4)
class Config:
copy_on_model_validation = False
BaseModel = declarative_base(cls=CommonBase) I also updated to pydantic 1.9.1 without change 🤔 |
1.9.1 hasn't changed Really I don't think it's correct for the sqlalchemy type to be raising an error on I think the best solution would be to catch errors like this in Something like def smart_deepcopy(obj: Obj) -> Obj:
"""
Return type as is for immutable built-in types
Use obj.copy() for built-in empty collections
Use copy.deepcopy() for non-empty collections and unknown objects
"""
obj_type = obj.__class__
if obj_type in IMMUTABLE_NON_COLLECTIONS_TYPES:
return obj # fastest case: obj is immutable and not collection therefore will not be copied anyway
try:
if not obj and obj_type in BUILTIN_COLLECTIONS:
# faster way for empty collections, no need to copy its members
return obj if obj_type is tuple else obj.copy() # type: ignore # tuple doesn't have copy method
except (TypeError, ValueError, RuntimeError):
# do we really dare to catch ALL errors? Seems a bit risky
pass
return deepcopy(obj) # slowest way when we actually might need a deepcopy What do you think? PR welcome, but I'm not sure when I'll get around to it - I'm currently flat out on pydantic-core getting ready for pydantic v2. But at least your issue has made me think about how we deal with deepcopy in v2 😄 . |
per their docs
So could this perhaps be fixed by changing the smart deep copy line to if obj is None and obj_type in BUILTIN_COLLECTIONS: Does this method expect other if (obj is None or not obj) and obj_type in BUILTIN_COLLECTIONS: |
That doesn't work, |
Isn't the immutable collection type check covering |
I'm not sure what you mean, but neither of your code suggestions will work. Best to try it and see what happens. |
You were right, my train of thought would have required a quite ugly double negative to work. I was only trying to avoid catching errors, but I suspect your proposed approach is the cleanest and safest. Will submit a pr |
I believe this is also the source of this issue I just filed against Starlite: litestar-org/litestar#333 There, the default behavior of The issue filed at Starlite has a minimal reproduction of attempting to instantiate a |
Checks
Bug
Output of
python -c "import pydantic.utils; print(pydantic.utils.version_info())"
:On import of this BaseModel, we are getting a hard to track down error that seems to be related to sqlalchemy boolean expressions, but stemming from the smart_deepcopy line in pydantic and I am not quite sure what the real issue is, and whether it is caused by pydantic, sqlalchemy, or my setup (most likely).
The stack trace received on import is:
I placed a debugger just before the raise and found that:
This is not new code in pydantic, and it also seems this check in sqlalchemy was added almost a decade ago so I am sure that is must be something wrong with either my expectatins or setup here. While investigating, it seems that this indeed is the correct way to setup a postgres UUID pk column within a pydantic model backed by sqlalchemy, but I am clearly missing something.
Thanks for the incredible work you do on this library!
The text was updated successfully, but these errors were encountered: