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

Adding _init_private_attributes to SQLModel __init__ function. #472

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

alexisgaziello
Copy link

Fixing:

Private attributes cannot be used in SQLModel. Attributes initialized with PrivateAttr cannot be found.

Proposed solution:

Add a missing line from Pydantic source code.

Note: the class method from_orm does have this initialization.

Related issues:

#149 mentions this issue

Disclaimer

This is my first-ever contribution to an open-source project. I would appreciate any kind of feedback.

@github-actions
Copy link

📝 Docs preview for commit 7a2b7af at: https://63519cbbc2f8c40065737d3f--sqlmodel.netlify.app

@codecov
Copy link

codecov bot commented Oct 20, 2022

Codecov Report

Base: 98.49% // Head: 98.50% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (7a2b7af) compared to base (ea79c47).
Patch coverage: 100.00% of modified lines in pull request are covered.

❗ Current head 7a2b7af differs from pull request most recent head 844e21c. Consider uploading reports for the commit 844e21c to get more accurate results

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #472   +/-   ##
=======================================
  Coverage   98.49%   98.50%           
=======================================
  Files         185      186    +1     
  Lines        5856     5867   +11     
=======================================
+ Hits         5768     5779   +11     
  Misses         88       88           
Impacted Files Coverage Δ
tests/test_private_attributes.py 100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@t1mk1k
Copy link

t1mk1k commented Nov 2, 2022

Thanks for the fix! I've used it as a workaround in my project by adding it to the init method of my own model.

I noticed that when I loaded a model from the database the private attributes were not initialized because the init in SQLModel is not called. I had to call it separately on the model before being able to use the private attribute.

Have you tested whether your fix works when loading a model from the database?

@alexisgaziello
Copy link
Author

Thanks for the fix! I've used it as a workaround in my project by adding it to the init method of my own model.

I noticed that when I loaded a model from the database the private attributes were not initialized because the init in SQLModel is not called. I had to call it separately on the model before being able to use the private attribute.

Have you tested whether your fix works when loading a model from the database?

Do you mean in an example such as the one below?

test_id = uuid4()


class Hero(SQLModel, table=True):
    id: UUID = Field(default_factory=uuid4, primary_key=True)
    _private_hero_attribute: str = PrivateAttr(default="private hero value")

    metadata = MetaData()


with Session(engine) as session:
    hero_rusty_man = Hero(
        id=test_id,
    )
    session.add(hero_rusty_man)
    session.commit()

with Session(engine) as session:
    statement = select(Hero).where(Hero.id == test_id)
    hero = session.exec(statement).scalars().first()

    assert hero._private_hero_attribute == "private hero value"  # this fails

    hero = Hero.from_orm(hero)
    assert hero._private_hero_attribute == "private hero value"  # this doesn't fail

Indeed, when running SQLAlchemy, the private attributes don't get correctly initialized.
Calling the method from_orm solves the problem for this particular case. Not ideal.

I haven't figured out if there is a way to "automate" this process. Maybe SQLAlchemy is using another init/factory_function?

@github-actions
Copy link

📝 Docs preview for commit 844e21c at: https://639ce0b204318b01e2823f64--sqlmodel.netlify.app

@lucasgadams
Copy link

Whats the status of this?

@lucas-labs
Copy link

Any chance we can get this merged?

@alexisgaziello
Copy link
Author

@lucas-labs the reality is that the fix doesn't fully work, as shown in #472 (comment)

@tiangolo tiangolo added the bug Something isn't working label Oct 22, 2023
@AAraKKe
Copy link

AAraKKe commented Feb 5, 2024

Calling the method from_orm solves the problem for this particular case. Not ideal.

This is the way this works in Pydantic; the method is called from init, from_orm, and construct; we need to call it from any place where initialization is required. Could you release this fix? We have the entire infrastructure from a base model from where all objects inherit from a BaseModel to make it much simpler to handle connections. Still, without the ability to use private attributes, this makes it impossible to handle.

I imagine this is the case for many users since a model that represents a database object might need to have more often attributes that do not necessary link to the database itself and are used for internal logic.

@sharenz
Copy link

sharenz commented Feb 10, 2024

I think I am also running into this issue. I am retrieving an object from the database and want to copy it like described in the fastapi docs for partial updates.

object_from_db = session.exec(select_cmd).one_or_none()

copied_object = object_from_db.model_copy() 

The model_copy results in the following error:
object has no attribute '__pydantic_extra__'. Did you mean: '__pydantic_private__'?

Is there a workaround I can use?

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

Successfully merging this pull request may close these issues.

None yet

7 participants