✨ Support SQLAlchemy keyed dict models #1287
Conversation
svlandeg
changed the title
|
Thanks, @svlandeg! |
|
Any update on this? This is a great feature add that I need. For now, I'm setting the requirement to use git at this PR. |
|
@svlandeg what is the next step in the process? How do I get this to be reviewed? (Thanks!) |
|
Hi @nkrishnaswami! It's in our review queue, so not much to be done by you at this point in time. We'll get back to you once we've had time to go through this in details 🙏 |
sqlmodel/_compat.py
Outdated
| # If a dict, then use the value type | ||
| elif origin is dict: | ||
| use_annotation = get_args(annotation)[1] |
There was a problem hiding this comment.
@nkrishnaswami, could you please take a look at this implementation?
https://github.com/fastapi/sqlmodel/pull/1631/files
I think it would be useful to handle situation when field is annotated as plain dict
Co-authored-by: Motov Yurii <[email protected]>
nkrishnaswami
force-pushed
the
allow-dict-relationship
branch
from
c7c1402 to
a801312
Compare
There was a problem hiding this comment.
@nkrishnaswami, thank you!
Could you please address my review comments?
| # typing.Dict throws if it receives the wrong number of type arguments, but dict | ||
| # (3.10+) does not; and Pydantic v1 fails to process models with dicts with no | ||
| # type arguments. | ||
| @pytest.mark.skipif(sys.version_info < (3, 10), reason="dict is not subscriptable") |
There was a problem hiding this comment.
| @pytest.mark.skipif(sys.version_info < (3, 10), reason="dict is not subscriptable") | |
| @needs_py310 |
| elif origin is dict or origin is Mapping: | ||
| args = get_args(annotation) | ||
| if len(args) >= 2: |
There was a problem hiding this comment.
| elif origin is dict or origin is Mapping: | |
| args = get_args(annotation) | |
| if len(args) >= 2: | |
| elif origin is dict: | |
| args = get_args(annotation) | |
| if len(args) == 2: |
Do we really need to support Mapping? If we do, we should add test for this case.
And, I think we should require exactly 2 arguments - is it valid to add more than 2 arguments to dict \ Dict?
| __tablename__ = "parents" | ||
|
|
||
| id: Optional[int] = Field(primary_key=True, default=None) | ||
| children_by_color: dict[()] = Relationship( |
There was a problem hiding this comment.
Could you please explain why not children_by_color: dict = Relationship...?
dict[()] looks strange..
5 participants
To add support for SQLAlchemy mapped collections, I'd like to add a case for
dictorigins toget_relationship_to. This PR does so, with a test case using a dictionary with collection classattribute_keyed_dict.