Conditional initialization of attributes

[roganov]

While trying to use attrs with sqlalchemy, I came across an issue that in some cases it's needed to initialize an attribute only if it was passed to constructor.

Basically, this code

@attr.attrs
class A:
   a = attr.ib(initialize_if_given=True)

should translate to this

CONDITIONAL_INIT = object()

class A:
    def __init__(a=CONDITIONAL_INIT):
        if a is not CONDITIONAL_INIT:
            self.a = a

[hynek]

Aren’t you asking for a magic default value? Uninitialized attributes are basically time bombs.

[RonnyPfannschmidt]

@roganov can you state the actual use case instead of the proposed solution?

[wsanchez]

I this the described behavior would have no attribute a on class A if no corresponding argument is given to the initializer. I don't think that's feasible, since we have the class attribute, and I also don't think that attrs should support every odd thing you can do in Python, so I think I'm with Hynek here.

@RonnyPfannschmidt is right though; without explaining the use case, it's hard to know whether this is an oversight of some sort.

[roganov]

My use case has to do with SQLAlchemy ORM, specifically with relationships. If, for example, relationship parent is set to None on a mapped instance, then parent_id will be updated to NULL in database even if parent_id was set in Python.

Please see following test case to understand the issue.

import attr
import sqlalchemy as sa
import typing as t

from sqlalchemy import create_engine
from sqlalchemy.orm import mapper, relationship, sessionmaker

@attr.attrs(cmp=False, auto_attribs=True)
class Parent:
    id: int


@attr.attrs(cmp=False, auto_attribs=True)
class Child:
    id: int
    parent_id: t.Optional[int] = None
    parent: t.Optional[Parent] = None


metadata = sa.MetaData()
parents = sa.Table(
    'parents',
    metadata,
    sa.Column('id', sa.Integer, primary_key=True),
)
mapper(Parent, parents)
children = sa.Table(
    'children',
    metadata,
    sa.Column('id', sa.Integer, primary_key=True),
    sa.Column('parent_id', sa.Integer, sa.ForeignKey('parents.id'), nullable=False),
)
mapper(Child, children, properties={
    'parent': relationship(Parent, uselist=False),
})

engine = create_engine('sqlite://', echo=True)
metadata.create_all(engine)
session = sessionmaker(bind=engine)()

parent = Parent(id=1)
session.add(parent)
session.flush()

# FK constraint violation here
# because `parent` attribute is set to `None` in constructor.
child = Child(id=1, parent_id=1)
session.add(child)
session.flush()

[roganov]

@hynek It is already possible to create a 'time bomb':

@attr.s
class A:
    a = attr.ib(init=False)

a = A()
a.a  # AttributeError

Or is it a bug?

[wsanchez]

I believe what @hynek was trying to express was that if you have an uninitialized attribute after you initialize your instance, that this is bad juju. So I'd say your example above is a bug in the program; I do think it's generally unsafe to have init=False and no default unless you are doing something in, say, __attrs_post_init__ to rectify that.

[wsanchez]

Whoops wrong button!

[RonnyPfannschmidt]

so the basic gist is that you have pairs of attributes that are managed by a 3rd party library,
and whats really needed is not conditional initialization, but something more like sqlalchemy mapper awareness

aka a attrs_sqlalchemy addon

[RonnyPfannschmidt]

@hynek : i think this use case also directly shows a weakness of the codegen approach, at mapper declaration time the class declaration is done and one is unable to fix the generated code by then

[hynek]

You call it a weakness, I call it the biggest strength. :)

And yes, this calls for an add-on that wraps @attr.s, sets unconditionally init=False and then generates the necessary __init__.

That said: I don’t know the ORM bits of SQLAlchemy well enough, but I’m gonna claim that it would be cleaner/nicer to be able to write:

UNSET = object()

@attr.attrs(cmp=False, auto_attribs=True)
class Child:
    id: int
    parent_id: t.Optional[int] = UNSET
    parent: t.Optional[Parent] = UNSET

Than doing voodoo with getattr/hasattr. That gives you much better introspection.

Are you sure it’s impossible?

[RonnyPfannschmidt]

@hynek correct mapper handling needs integration with real descriptors for stuff like lazy loading of dependencies or deferred loading of properties - i dare to claim that attrs biggest "strength" makes it fundamentally incompatible with reasonable orm's

[roganov]

@hynek
Could you clarify what this UNSET thing helps to achieve? Also it won't typecheck.