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

Avoid __dict__ and __weakref__ attributes in AnyUrl and _BaseAddress subclasses #2890

Merged
merged 3 commits into from Aug 4, 2022
Merged

Conversation

nuno-andre
Copy link
Contributor

@nuno-andre nuno-andre commented Jun 8, 2021

Change Summary

Add an empty __slots__ declaration to AnyUrl and _BaseAddress subclasses to avoid the creation of __dict__ and __weakreef__ attributes. When inheriting from a class with __slots__, child subclasses will get a __dict__ and __weakref__ unless they also define __slots__.

from pydantic import AnyUrl, AnyHttpUrl

class AnyOtherHttpUrl(AnyUrl):
    allowed_schemes = {'http', 'https'}
    __slots__ = ()

print(set(dir(AnyHttpUrl)) - set(dir(AnyUrl)))
#> {'__weakref__', '__dict__'}

print(set(dir(AnyOtherHttpUrl)) - set(dir(AnyUrl)))
#> set()

Related issue number

None.

Checklist

  • Unit tests for the changes exist
  • Tests pass on CI and coverage remains at 100%
  • Documentation reflects the changes where applicable
  • changes/<pull request or issue id>-<github username>.md file added describing change
    (see changes/README.md for details)
  • My PR is ready to review, please add a comment including the phrase "please review" to assign reviewers

…subclasses

When inheriting from a class with `__slots__`, [child subclasses will get a `__dict__` and `__weakref__` unless they also define `__slots__`](https://docs.python.org/3/reference/datamodel.html#notes-on-using-slots).

```python
from pydantic import AnyUrl, AnyHttpUrl

class AnyOtherHttpUrl(AnyUrl):
    allowed_schemes = {'http', 'https'}
    __slots__ = ()

print(set(dir(AnyHttpUrl)) - set(dir(AnyUrl)))
#> {'__weakref__', '__dict__'}

print(set(dir(AnyOtherHttpUrl)) - set(dir(AnyUrl)))
#> set()
```
@PrettyWood
Copy link
Member

Hi @nuno-andre
What's the matter with having __dict__ and __weakref__?
What problem does it solve?
AFAIR I haven't seen any issue related to this 🤔

@nuno-andre
Copy link
Contributor Author

Hi @PrettyWood!

One of the __slots__'s purposes is to deny the creation of __dict__ to save memory.

from pydantic import AnyUrl, AnyHttpUrl
from pympler.asizeof import asizeof


class AnyOtherHttpUrl(AnyUrl):
    allowed_schemes = {'http', 'https'}
    __slots__ = ()


a = AnyUrl.__new__(AnyHttpUrl, 'https://example.com')
print(asizeof(a))
#> 224

b = AnyUrl.__new__(AnyOtherHttpUrl, 'https://example.com')
print(asizeof(b))
#> 120
from pydantic import BaseModel


class A(BaseModel):
    url: AnyHttpUrl


class B(BaseModel):
    url: AnyOtherHttpUrl


a = A(url='https://example.com')
print(asizeof(a))
#> 808

b = B(url='https://example.com')
print(asizeof(b))
#> 704

I think this is a pretty straightforward and valuable optimization and also makes a more consistent type model.

Copy link
Member

@samuelcolvin samuelcolvin left a comment

Choose a reason for hiding this comment

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

Seems good to me, I can't see how this will break anyone's code. But I'm sure we'll have someone shouting at us about breaking code in a few weeks from some weird usage.

changes/2890-nuno-andre.md Outdated Show resolved Hide resolved
@samuelcolvin samuelcolvin enabled auto-merge (squash) August 4, 2022 11:41
@samuelcolvin samuelcolvin merged commit bd0a28d into pydantic:master Aug 4, 2022
@tl24
Copy link

tl24 commented Aug 31, 2022

Let the shouting begin... 😃 We had a breaking change as we were reading the slots to be able to clone a RedisDsn and add in a password that was stored in a secret. We worked around by hard-coding the property names, but is there a better way of doing that?

For reference:

def copy_dsn(original, **override_args):
    # object properties don't show up with normal methods,
    # so we need to use the __slots__ property to get field names
    attrs = {n: getattr(original, n) for n in original.__slots__ if getattr(original, n) is not None}
    return type(original)(None, **{**attrs, **override_args})

dsn_with_password = copy_dsn(RedisDsn("redis://localhost:6379/0", password="secret")

@tl24
Copy link

tl24 commented Aug 31, 2022

I don't know if this would be a frequent use case, but something like copy on BaseModel would be nice:

dsn_with_password = RedisDsn("redis://localhost:6379/0").copy(update={"password":"secret"})

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants