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

Why descriptor don't work with attr? #881

Open
jalvespinto opened this issue Dec 6, 2021 · 13 comments
Open

Why descriptor don't work with attr? #881

jalvespinto opened this issue Dec 6, 2021 · 13 comments

Comments

@jalvespinto
Copy link

jalvespinto commented Dec 6, 2021

I am reading Architecture Patterns with Python and at some point the frozen dataclasses just don't work with sqlalchemy, so the author suggests setting unsafe_hash=True. To avoid making the class hashable I am using the descriptor bellow. I was also using @attr.s instead of @dataclass to get some of the features it has, like validators, but for some reason the descriptor works fine with dataclass, but not with attr and I can't understand why and if there is a way to make it work. If I set init=False on the attr.s class the descriptor will work for self.vid the I am only setting inside the init, but don't work with street and number.

from dataclasses import dataclass
import attr
import uuid


class Frozen:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = '_' + name

    def __set__(self, instance, value):
        if hasattr(instance, self.storage_name):
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            setattr(instance, self.storage_name, value)

    def __get__(self, instance, objtype=None):
        return getattr(instance, self.storage_name)

# Frozen descriptor works fine here
@dataclass
class Address:

    vid = Frozen()
    street = Frozen()
    number = Frozen()

    street: str
    number: str

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex

# Frozen descriptor don't work here
@attr.s
class Address:

    vid = Frozen()
    street = Frozen()
    number = Frozen()

    street: str = attr.ib
    number: str = attr.ib

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex
@hynek
Copy link
Member

hynek commented Dec 6, 2021

Try setting @attr.s(auto_attribs=True) (or even better: use @attr.define) and dropping the = attr.ib. You're overwriting both street and number this way.

Note that you can just use @attr.s(frozen=True) or @attr.frozen to get a frozen class.

@jalvespinto
Copy link
Author

jalvespinto commented Dec 6, 2021

The problem with frozen is that it does not work well with sqlalchemy for some reason, at least the way I am using it (cosmicpython/code#17).

The code bellow works. Its works with sqlalchemy and the descriptors will prevent setting the attributes. But if I use @attr.define I will get in __setattr__ _obj_setattr(self, name, nval) AttributeError: 'Address' object has no attribute '_sa_instance_state'. The same happens if I use frozen=True in both dataclass or attr.

import uuid
from dataclasses import dataclass

import attr
from sqlalchemy import Column, Table, create_engine
from sqlalchemy.orm import registry,sessionmaker
from sqlalchemy.sql.sqltypes import Integer, String


class FrozenLeadingUnder:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = '_' + name

    def __set__(self, instance, value):
        if hasattr(instance, self.storage_name):
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            setattr(instance, self.storage_name, value)

    def __get__(self, instance, objtype=None):
        return getattr(instance, self.storage_name)


class Frozen:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = name

    def __set__(self, instance, value):
        if self.storage_name in instance.__dict__:
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            instance.__dict__[self.storage_name] = value

    def __get__(self, instance, objtype=None):
        try:
            return instance.__dict__[self.storage_name]
        except KeyError:
            raise KeyError(f'No {self.storage_name} attribute found.')


mapper_registry = registry()


@dataclass
class Address:

    vid = FrozenLeadingUnder()
    street = Frozen()
    number = Frozen()

    street: str
    number: str

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex


addresses = Table(
    'addresses', mapper_registry.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('street', String(255)),
    Column('number', String(255))
)


def start_mappers():
    mapper_registry.map_imperatively(Address, addresses)


if __name__ == "__main__":

    engine = create_engine("sqlite:///:memory:")
    mapper_registry.metadata.create_all(engine)
    start_mappers()
    session = sessionmaker(bind=engine)()

    ad = Address("Av", '123')

    session.add(ad)
    session.commit()

@hynek
Copy link
Member

hynek commented Dec 6, 2021

Ah you're using additional attributes. Try @attr.define(slots=False) then.

@jalvespinto
Copy link
Author

jalvespinto commented Dec 6, 2021

But even without them, the descriptors won't work. The behavior I am seeing is that any field I declare will override the descriptor, so the descriptor will not work. If I don't declare the fields and just set them on init the descriptor will work, but I don't see the attribute when invoking the instance:

@attr.define
class Address:

    street = Frozen()
    number = Frozen()

    def __init__(self, street: str, number: str) -> None:
        self.street = street
        self.number = number

>>> a = Address('bla','ble')
>>> a.street = 'dfad'
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/usr/test.py", line 35, in __set__
    raise AttributeError(
AttributeError: readonly attribute str
>>> a
Address()

@hynek
Copy link
Member

hynek commented Dec 6, 2021

You have to annotate street and number if attrs is to find them.

@jalvespinto
Copy link
Author

jalvespinto commented Dec 6, 2021

yes, but in that case the descriptor will not work:

>>> a = Address('ba','be')
>>> a.street = 'fadfadf'
>>> a
Address(street='fadfadf', number='be')
>>> 

@hynek
Copy link
Member

hynek commented Dec 6, 2021

I have tried:

@attr.define(slots=False)
class Address:

    street: str = Frozen()
    number: str = Frozen()
    vid: str = Frozen()

And I get:

In [2]: a = Address("x", "y", "z")

In [3]: a
Out[3]: Address(street='x', number='y', vid='z')

In [4]: a.street = "asdf"
---------------------------------------------------------------------------
AttributeError                            Traceback (most recent call last)
<ipython-input-4-f728b241b7ae> in <module>
----> 1 a.street = "asdf"

~/FOSS/attrs/t.py in __set__(self, instance, value)
     12     def __set__(self, instance, value):
     13         if hasattr(instance, self.storage_name):
---> 14             raise AttributeError(
     15                 f'readonly attribute {value.__class__.__name__}')
     16         else:

AttributeError: readonly attribute str

The attribute name is wrong, but it seems to work otherwise?

@euresti
Copy link
Contributor

euresti commented Dec 6, 2021

So I'm wondering if this is a typo?

# Frozen descriptor don't work here
@attr.s
class Address:
    <snip>
    street: str = attr.ib

That should in theory be attr.ib(). otherwise attr.s is doing nothing with that and it's just overwriting the Frozen that's going on above.

@jalvespinto
Copy link
Author

oh, I see. I was doing two different assignments to the same name, one for the descriptor and another for the actual attributes, just like the way I was doing with dataclass. I wonder why that works with dataclass and not with attrs. It seems slots=False will do it. I will refactor everything and then come here to let you know how it went, for good or bad. tks

@euresti
Copy link
Contributor

euresti commented Dec 6, 2021

Note that this:

@attr.define(slots=False)
class Address:
    street: str = Frozen()
    number: str = Frozen()
    vid: str = Frozen()

has a problem where attrs thinks that Frozen() is a default value.
So it lets you do this:

>>> Address()
Address(street=<foo.Frozen object at 0x7fab78034df0>, number=<foo.Frozen object at 0x7fab78034ca0>, vid=<foo.Frozen object at 0x7fab78034e20>)
>>> 

So you'll probably want to write your own __init__

@attr.define(slots=False)
class Address:
    street: str = Frozen()
    number: str = Frozen()
    vid: str = Frozen()

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex

@jalvespinto
Copy link
Author

noted. tks

@jalvespinto
Copy link
Author

jalvespinto commented Dec 6, 2021

I am having some difficulties still. Now I had to add some more code to my example. I included a House class that has an Address as attribute and also some another Table and mapper for this new class.

import uuid
from dataclasses import dataclass

import attr
from sqlalchemy import Column, Table, ForeignKey, create_engine
from sqlalchemy.orm import registry, relationship, sessionmaker
from sqlalchemy.sql.sqltypes import Integer, String


class FrozenPrivate:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = '_' + name

    def __set__(self, instance, value):
        if hasattr(instance, self.storage_name):
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            setattr(instance, self.storage_name, value)

    def __get__(self, instance, objtype=None):
        return getattr(instance, self.storage_name)


class Frozen:
    """cannot change attribute if it already exists"""

    def __set_name__(self, owner, name):
        self.storage_name = name

    def __set__(self, instance, value):
        if self.storage_name in instance.__dict__:
            raise AttributeError(
                f'readonly attribute {value.__class__.__name__}')
        else:
            instance.__dict__[self.storage_name] = value

    def __get__(self, instance, objtype=None):
        try:
            return instance.__dict__[self.storage_name]
        except KeyError:
            raise KeyError(f'No {self.storage_name} attribute found.')


mapper_registry = registry()


@attr.define(slots=False)
# @dataclass
class Address:

    street: str = Frozen()
    number: str = Frozen()
    vid: str = FrozenPrivate()

    def __init__(self, street: str, number: str, vid=None) -> None:
        self.street = street
        self.number = number
        self.vid = vid if vid else uuid.uuid4().hex


class House:

    def __init__(self, ad: Address) -> None:
        self.address = ad


houses = Table(
    'houses', mapper_registry.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('address_id', ForeignKey('addresses.id'))
)

addresses = Table(
    'addresses', mapper_registry.metadata,
    Column('id', Integer, primary_key=True, autoincrement=True),
    Column('street', String(255)),
    Column('number', String(255))
)


def start_mappers():
    mapper_registry.map_imperatively(Address, addresses)
    mapper_registry.map_imperatively(
        House,
        houses,
        properties={'address': relationship(Address, backref='house')}
    )


if __name__ == "__main__":

    engine = create_engine("sqlite:///:memory:")
    mapper_registry.metadata.create_all(engine)
    start_mappers()
    session = sessionmaker(bind=engine)()

    ad = Address("Av", '123')
    h = House(ad)

    session.add(h)
    session.commit()

    search = session.query(House).join(House.address).filter(
        Address.street == ad.street and Address.number == ad.number).first()

    print(search.address)

It seems that the filter method don't recognize the ad variable as an Address instance:

Traceback (most recent call last):
  File "/home/jap/projects/vlep/app/bora.py", line 107, in <module>
    Address.street == ad.street and Address.number == ad.number).first()
  File "/home/jap/projects/vlep/app/bora.py", line 42, in __get__
    return instance.__dict__[self.storage_name]
AttributeError: 'NoneType' object has no attribute '__dict__'

I tried to simple delete the get method on the descriptor, but them the descriptor will not work (the query will, but not the descriptor):

Python 3.9.7 (default, Sep  9 2021, 23:20:13) 
[GCC 9.3.0] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import bora as b
>>> ad = b.Address('fdaf','fadfa')
>>> ad.street = 'fdfadfafa'
>>> ad
Address(street='fdfadfafa', number='fadfa', vid='9e032ff432844679a03ebc4010865c00')

@hynek
Copy link
Member

hynek commented Dec 12, 2021

As a random though: I think you could use field hooks to remove Frozen as the default value: https://www.attrs.org/en/stable/extending.html#automatic-field-transformation-and-modification

maybe they could be useful to you in general.

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

No branches or pull requests

3 participants