Codecov Report

Merging #712 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #712   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          15     15           
  Lines        2727   2721    -6     
  Branches      539    539           
=====================================
- Hits         2727   2721    -6

I noticed you are saying in the history update that removing __getattr__ is a breaking change, but I can't think of any code it could break (given object.__getattr__ would still be present.) Am I missing something? I guess removing __values__ is still a breaking change, maybe better to say that in the history

Unfortunately object doesn't have __getattr__:

>>> object.__getattr__                                                                          
Traceback (most recent call last)
...
AttributeError: type object 'object' has no attribute '__getattr__'

So any some code that used BaseModel.__getattr__ will fail may work strange:

from typing import Dict, Any
from pydantic import BaseModel

class MyModel(BaseModel):
    my_values: Dict[str, Any]

    def __getattr__(self, item):
        try:
            return self.my_values[item]
        except KeyError:
            return super().__getattr__(item)

m = MyModel(my_values=dict(foo='bar'))
print(m.my_values, m.foo, sep='\n')
m.bar  # cause AttributeError
...
{'foo': 'bar'}
bar

Traceback (most recent call last):
    ...
    return self.my_values[item]
KeyError: 'bar'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
    ...
    return super().__getattr__(item)
AttributeError: 'super' object has no attribute '__getattr__'

But having thoughts about this: it will fail only if object actually doesn't have attribute (nor in __dict__ or my_values), so possibly it will not break anything at all, except for error message will be different and will point not to attribute that caused the problem, but to __getattr__

As for turning __values__ into a poperty (not removing it completely) , I can't imagine code it could break either. Any suggestions?

D'oh good catch on both points. I think what I was thinking (😅) was that getattr(obj, "key") would still work. Theoretically I guess it's possible someone could be using model.__getattr__("x") in their code, so maybe it is breaking. (This would require you to call getattr(model, "x") instead, but that should probably be used anyway).

So, maybe worth the call out, but unless we're missing something I think this seems very unlikely to affect existing code.

@dmontagu, just updated #712 (comment) with 'real code' example, check it out :)

@MrMrRobat
I feel good the changed code 👍

This also fixes #702 / #701 could you add this test from there:

def test_init_inspection():
    class Foobar(BaseModel):
        x: int

        def __init__(self, **data) -> None:
            with pytest.raises(AttributeError):
                assert self.x
            super().__init__(**data)

    Foobar(x=1)

Then this is ready to merge.

Just saw your comment here, thanks for adding it!

Just saw your comment here, thanks for adding it!

No problem, I was working on a script to easily pull from and push to other's pull requests, so wanted to try it. :-)

Thanks so much for finding an implementing this.