-
-
Notifications
You must be signed in to change notification settings - Fork 688
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
✨ Add support for hybrid_property #482
base: main
Are you sure you want to change the base?
Conversation
📝 Docs preview for commit 55c9526 at: https://635fb362686fae1e8384a5e3--sqlmodel.netlify.app |
📝 Docs preview for commit f5a474f at: https://635fb41dc6090b1e80805996--sqlmodel.netlify.app |
The proposed addition was working correctly for |
📝 Docs preview for commit a7ec9ff at: https://6362568c806a400cb061aacd--sqlmodel.netlify.app |
Codecov ReportBase: 98.49% // Head: 98.50% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #482 +/- ##
=======================================
Coverage 98.49% 98.50%
=======================================
Files 185 186 +1
Lines 5856 5890 +34
=======================================
+ Hits 5768 5802 +34
Misses 88 88
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. |
📝 Docs preview for commit 32b125c at: https://6362577741a93d01dcd99ece--sqlmodel.netlify.app |
📝 Docs preview for commit 1359098 at: https://636258b54b0bfa040aa9faf9--sqlmodel.netlify.app |
📝 Docs preview for commit 93509eb at: https://639ce098ecf1fd03689b400e--sqlmodel.netlify.app |
Any plans to solve it? |
Hi, it has been fixed in the current PR implementation. I had forgotten to add a comment about it. |
@van51 Any plans to merge it? I use your commit |
Hi @shepilov-vladislav , unfortunately I don't have write access to this repository. We have to ping/mention someone from the maintainers. I am glad to know that it works for you though :) |
I see 😢 @tiangolo Can you help with this? What can we do to force this? |
1 similar comment
I see 😢 @tiangolo Can you help with this? What can we do to force this? |
This hybird_property is cool ,it solved my pain,hope to merge |
works great. hope to merge too. |
Would love to have this |
Bumping this as it really should be pulled & merged. |
@tiangolo after new releases with sqlalchemy 2.0 support, maybe it's time to merge this? |
Willy Wonka: But Charlie, don’t forget what happened to the man who suddenly got everything he always wanted. Please sir, merge and release. |
Use my PR #801 if you need these features |
@tiangolo Any ETA on this? Would be really appreciated 😊 |
Good god, nearly 2-years and @tiangolo cant even be bothered to acknowledge this major shortcoming with sqlmodel? And this is even a PR vs just a complaint in an issue? Damn. I guess this is a prime example of why we are quickly reverting our short-lived idea about migrating our codebade to SQLModel. |
@tiangolo any comment on this? |
Honestly it's probably best to just kill the PR. This is a dead project. Hopefully author has enough courtesy to archive it without wasting anymore dev's time. Understandably, he probably got carried away with repackaging useful libraries with typing and pydantic... given the success of FastAPI... but SQLAlchemy was too big and complex of a library of keep up with, and largely took care of typing issues on their own. |
Attempt to solve #299 and add support for
hybrid_property
properties.