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

Allow for passing keyword arguments to from_orm #3375

Closed
wants to merge 14 commits into from
Closed
1 change: 1 addition & 0 deletions changes/3375-dosisod.md
@@ -0,0 +1 @@
Allow for passing keyword arguments to `from_orm`
30 changes: 30 additions & 0 deletions docs/examples/models_orm_mode_kwargs.py
@@ -0,0 +1,30 @@
from pydantic import BaseModel
import sqlalchemy as sa
from sqlalchemy.ext.declarative import declarative_base


class MyModel(BaseModel):
dosisod marked this conversation as resolved.
Show resolved Hide resolved
foo: str
bar: int
spam: bytes

class Config:
orm_mode = True


BaseModel = declarative_base()


class SQLModel(BaseModel):
__tablename__ = 'my_table'
id = sa.Column('id', sa.Integer, primary_key=True)
foo = sa.Column('foo', sa.String(32))
bar = sa.Column('bar', sa.Integer)


sql_model = SQLModel(id=1, foo='hello world', bar=123)

pydantic_model = MyModel.from_orm(sql_model, bar=456, spam=b'placeholder')

print(pydantic_model.dict())
print(pydantic_model.dict(by_alias=True))
8 changes: 8 additions & 0 deletions docs/usage/models.md
Expand Up @@ -152,6 +152,14 @@ _(This script is complete, it should run "as is")_
The example above works because aliases have priority over field names for
field population. Accessing `SQLModel`'s `metadata` attribute would lead to a `ValidationError`.

You can also achieve the same thing by passing keyword arguments to `from_orm`, instead of
using Field aliases:

```py
{!.tmp_examples/models_orm_mode_kwargs.py!}
```
_(This script is complete, it should run "as is")_
Comment on lines +158 to +161
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dosisod, this should fix docs build:

Suggested change
```py
{!.tmp_examples/models_orm_mode_kwargs.py!}
```
_(This script is complete, it should run "as is")_
{!.tmp_examples/models_orm_mode_kwargs.md!}

You can test this locally as well, but you will need to pull changes from remote.


### Recursive ORM models

ORM instances will be parsed with `from_orm` recursively as well as at the top level.
Expand Down
8 changes: 4 additions & 4 deletions pydantic/main.py
Expand Up @@ -568,10 +568,10 @@ def parse_file(
return cls.parse_obj(obj)

@classmethod
def from_orm(cls: Type['Model'], obj: Any) -> 'Model':
def from_orm(cls: Type['Model'], obj: Any, **kwargs: Any) -> 'Model':
if not cls.__config__.orm_mode:
raise ConfigError('You must have the config attribute orm_mode=True to use from_orm')
obj = {ROOT_KEY: obj} if cls.__custom_root_type__ else cls._decompose_class(obj)
obj = {ROOT_KEY: obj} if cls.__custom_root_type__ else cls._decompose_class(obj, kwargs)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like, kwargs will be silently ignored if cls.__custom_root_type__ is true. I don't think it should be the case.

m = cls.__new__(cls)
values, fields_set, validation_error = validate_model(cls, obj)
if validation_error:
Expand Down Expand Up @@ -698,10 +698,10 @@ def validate(cls: Type['Model'], value: Any) -> 'Model':
return cls(**value_as_dict)

@classmethod
def _decompose_class(cls: Type['Model'], obj: Any) -> GetterDict:
def _decompose_class(cls: Type['Model'], obj: Any, kwargs: Dict[str, Any]) -> GetterDict:
if isinstance(obj, GetterDict):
return obj
return cls.__config__.getter_dict(obj)
return cls.__config__.getter_dict(obj, kwargs)
Copy link
Contributor

@Bobronium Bobronium Aug 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a breaking change and should be reflected in docs and history.


@classmethod
@no_type_check
Expand Down
12 changes: 10 additions & 2 deletions pydantic/utils.py
Expand Up @@ -413,18 +413,26 @@ class GetterDict(Representation):
We can't inherit from Mapping[str, Any] because it upsets cython so we have to implement all methods ourselves.
"""

__slots__ = ('_obj',)
__slots__ = ('_obj', '_kwargs')

def __init__(self, obj: Any):
def __init__(self, obj: Any, kwargs: Optional[Dict[str, Any]] = None):
self._obj = obj
self._kwargs = kwargs or None

def __getitem__(self, key: str) -> Any:
try:
if self._kwargs and key in self._kwargs:
return self._kwargs[key]
Comment on lines +424 to +425
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, this should be out of try/except AttrubuteError block.

And perhaps we should ask for forgiveness, not permission (like you've done it in get method)?

return getattr(self._obj, key)
except AttributeError as e:
raise KeyError(key) from e

def get(self, key: Any, default: Any = None) -> Any:
if self._kwargs:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMO, The way of getting key out of kwargs should match one in __getitem__

try:
return self._kwargs[key]
except KeyError:
pass
return getattr(self._obj, key, default)

def extra_keys(self) -> Set[Any]:
Expand Down
50 changes: 48 additions & 2 deletions tests/test_orm_mode.py
Expand Up @@ -26,7 +26,7 @@ def __getattr__(self, key):
raise AttributeError()

t = TestCls()
gd = GetterDict(t)
gd = GetterDict(t, {'extra': 42})
assert gd.keys() == ['a', 'c', 'd']
assert gd.get('a') == 1
assert gd['a'] == 1
Expand All @@ -46,6 +46,7 @@ def __getattr__(self, key):
assert len(gd) == 3
assert str(gd) == "{'a': 1, 'c': 3, 'd': 4}"
assert repr(gd) == "GetterDict[TestCls]({'a': 1, 'c': 3, 'd': 4})"
assert gd['extra'] == 42


def test_orm_mode_root():
Expand Down Expand Up @@ -253,7 +254,7 @@ class TestCls:
x = 1
y = 2

def custom_getter_dict(obj):
def custom_getter_dict(obj, _):
assert isinstance(obj, TestCls)
return {'x': 42, 'y': 24}

Expand Down Expand Up @@ -357,3 +358,48 @@ class Config:

# Pass dictionary data directly
State(**{'user': {'first_name': 'John', 'last_name': 'Appleseed'}})


def test_orm_mode_with_kwargs():
class MyModel(BaseModel):
foo: str
bar: int
spam: bytes

class Config:
orm_mode = True

class SQLModel(BaseModel):
id: int
foo: str
bar: int

sql_model = SQLModel(id=1, foo='hello world', bar=123)
pydantic_model = MyModel.from_orm(sql_model, bar=456, spam=b'placeholder')

assert pydantic_model.foo == 'hello world'
assert pydantic_model.bar == 456
assert pydantic_model.spam == b'placeholder'


def test_orm_mode_with_kwargs_property_wont_be_acessed():
class Model(BaseModel):
a: int

class Config:
orm_mode = True

property_was_accessed = False

class Test:
a = 1

@property
def test(self):
nonlocal property_was_accessed
property_was_accessed = True

test = Test()
Model.from_orm(test, extra=42)

assert property_was_accessed is False