Attrs: attr.Factory is a little wordy

[glyph]

attrs is a huge net win on an object like this:

@attr.s
class Point(object):
    def __init__(self, x, y, z):
        self.x = x
        self.y = y
        self.z = z

import attr
@attr.s
class Point(object):
    x = attr.ib()
    y = attr.ib()
    z = attr.ib()

but it's a lot less clear when you have something like this:

class Cache(object):
    def __init__(self):
        self._stored = []
        self._by_name = {}
        self._by_id = {}

which becomes

@attr.s
class Cache(object):
    _stored = attr.ib(default=attr.Factory(list))
    _by_name = attr.ib(default=attr.Factory(dict))
    _by_id = attr.ib(default=attr.Factory(dict))

I think an alias for this behavior, like:

@attr.s
class Cache(object):
    _stored = attr.ib(new=list)
    _by_name = attr.ib(new=dict)
    _by_id = attr.ib(new=dict)

could make initializing these types of mutable objects a lot less verbose.

[Tinche]

Don't really have a strong opinion here, but I've been finding myself really needing factories that take self lately. (I'm getting along with using __attrs_post_init__ but it's kind of a workaround...)

If we're changing the design it'd be nice for the new design to elegantly accommodate these kinds of callables too.

[hynek]

I think it should at least be factory if anything. But I have to admit that the reason it’s wordy is that I wanted to avoid mutually exclusive arguments.

Sadly, that ship has sailed long ago so keeping it up doesn’t make a lot of sense anymore.

---

I guess passing a half-initialized self into the factory would make a lot of people happy…

[AngelEzquerra]

I also think that attr could make it easier to set default mutable attribute values. IMHO having to explicitly create a Factory object is too wordy and not very discoverable. Thus I like this proposal. In fact I proposed something similar to Hynek via email, although I suggested "factory" rather than "new" for the keyword argument. I think using "factory" rather than "new" would make it more clear than a factory is introduced automatically.

That being said, I think that while this would be nice it would only be a partial solution. It would not make it easy enough to set an attribute default value to a non empty list. To do that currently you need to use a Factory and a lambda:

~Python
@attr.s
class Player(object):
position = attr.ib(default=attr.Factory(lambda: [0, 0, 0]))
~

IMHO that is way too verbose, complex and obscure for such as common use case. With this new proposal you would do:

~Python
@attr.s
class Player(object):
position = attr.ib(factory=lambda: [0, 0, 0])
~

which is better, but still a bit obscure.

Instead, or perhaps in addition to the "factory" parameter there should be a simple way to set an attribute default value to a mutable, non empty value. For example there could be a new "mdefault" attribute (or perhaps "new", if we use "factory" for the previous use case) which would both add the Factory and the lambda for you:

~Python
@attr.s
class Player(object):
position = attr.ib(new=[0, 0, 0])
~

Ideally attr may be smart enough to detect when the value is immutable and skip the lambda when not needed.

[Tinche]

Hm, I see @hynek 's point. I don't actually need self, but I do need the values of other arguments (post-convert, ideally post-validation). There's already an issue for this over at https://github.com/python-attrs/attrs/issues/165.

First of all, I like attr.ib(factory=<callable>). Second, we could run with this and inspect the callable arguments. For every argument, inject the value of the attribute with the same name. No arguments - just call the callable.

@attr.s
class A:
    a = attr.ib(convert=int)
    b = attr.ib(factory=lambda a: a+1)


# Generated __init__ equivalent:
def __init__(self, a, b):
    self.a = int(a)
    self.b = (lambda a: a+1)(self.a)

A small problem with this is runtime errors:

@attr.s
class A:
    a = attr.ib()
    b = attr.ib(factory=lambda c: c+1)
    c = attr.ib()

This should be an error. But we kinda already do this by throwing errors if attributes with default aren't listed last.

[glyph]

On the one hand, I love this because it is incredibly precise and doesn't carry the structural risk of "here's half a self I found lying around in the garbage, do stuff to it, I'm sure it's fine" that you have with __init__.

On the other hand, wow, what terrifying magic. Where did a come from???

Finally though - there's no problem with putting c into b's arg list, as long as there are no cycles in the graph, we can just topologically sort all attributes by their dependencies and then set them in reverse dependency order in the generated __init__ 😈

[hynek]

Have you guys lost your minds? 😱

[glyph]

Have you guys lost your minds? 😱

You're right, if brevity is the goal, a kwarg is probably not sufficiently terse to be worthwhile. How about this:

@attr.s
class X:
    a = attr.ib() | None # -> `default=`
    b = attr.ib() ^ list # -> `default=Factory(list)`
    c = attr.ib() & [] # -> `default=Factory(lambda shared_list=[]: copy.deepcopy(shared_list))

Thoughts?

[glyph]

Oh wait, perhaps I missed the obvious here:

[glyph]

@attr.s
class X:
    a = attr.ib() + None # -> `default=None`
    b = attr.ib() * list # -> `default=Factory(list)`
    c = attr.ib() ** [] # -> `default=Factory(lambda shared_list=[]: copy.deepcopy(shared_list))

[hynek]

Silliness aside, if we’d want to pass optionally a half-initialized self, we’d have to distinguish between methods whose only argument happens to be self and functions that take a self. So I guess we need to use a unique name? half_initialized_self? 😂

Seems fragile and implicit though…

[hynek]

Did we kind of agree on attr.ib(default=attr.Factory(list)) == attr.ib(factory=list)?

[hynek]

(if you need self, you can always use the @x.default decorator)

[glyph]

A lot of people really dislike the word "factory"; it has some baggage. How do you feel about something even briefer, like attr.ib(new=list)?

[hynek]

I appreciate the marketing angle but we really want to add a second name for the same concept -- a name that doesn't really make a lot of sense? :-/

Don't get me wrong: I'd love to call it new I just have a very hard time to reconcile/rationalize it.

[glyph]

Factory is a pattern for an object with methods that produces other objects with methods. But the more general concept of making a new thing, or creating something, is far more general than a factory. For me it makes sense that Factory would be the class, but that the non-Factory way of specifying it would be under a different name.

[wsanchez]

glyph has a far tighter definition of "factory" than I've ever heard. I think that name is fine, FWIW.

Though oddly, I associate "new" with other languages (Java) and totally think of that as a thing that produces an object with methods. But then… isn't everything an object with methods in Python? SIGLOOP

[glyph]

There's some debate; it's certainly used in other ways too. I didn't mean to say that my definition was precisely technically correct, just that the term has more of a technical connotation due to the design-patterns literature.

[hynek]

1. For me, a factory is something that poops out instances and isn’t a constructor/initializer.
2. I have no problem to associate new with creating new objects – you guys are forgetting the “default” part of it. :) To that end, factory isn’t great either I have to concede. default_new would probably be best I think – but y’all are too lazy to type that, aren’t you? :-|

What I’m thinking about right now is that someone who isn’t intimate with attrs comes to a code base and new seems like something _super_ confusing. And being hard on code readers has always been a popular point to bash attrs for so I would prefer to not fuel that any more.

Would it kill y’all to have:

def clever_factory(inst):
    ...
    return value

@attr.s
class C:
    x = attr.ib(default_new=list)
    y = attr.ib(default_new(attr.takes_self(clever_factory))
    z = attr.ib(default=42)

?

To me, this seems like a sweet spot for consistency, brevity, and clarity. Comments? 🙈

[glyph]

For the middle one, what about attr.ib(default=Factory(clever_factory, pass_self=True))?

[hynek]

That’s a great idea @glyph! I love the idea so much that I built a time machine and implemented it in the past, so it can be in 17.1! :D

(but good you made me re-look it up; it’s takes_self not pass_self).

[Tinche]

At this point I feel like the easiest way of solving this is just changing the docs to:

from attr import Factory as new

@attr.s
class C:
    x = attr.ib(default=new(list))
    y = attr.ib(default=new(clever_factory, takes_self=True))
    z = attr.ib(default=42)

😎

[wsanchez]

@Tinche… uh… dammit that's too easy!

[glyph]

This is actually what I do in my own code :).

[hynek]

So um after typing attr.ib(default=attr.Factory(list)) 8x for a single class…am I the only one having weird feelings about new=? Suddenly I’m sympathetic but I don’t want to make decisions out of recent pain.

[glyph]

give in to your anger

[hynek]

OK but I just can’t get over the inconsistency of new/factory. That would bother me until my death.

Would you consider attr.ib(factory=list) a sufficiently significant improvement?

[wsanchez]

@Tinche's example is pretty concise, even if you type Factory instead of new, so this seems unnecessary… I agree with your desire to void mutually exclusive arguments.

[hynek]

Well, we could go as far as adding a factory for Factory called new. 🤣

[glyph]

I do own factoryfactoryfactory.com

[aldanor]

attr.ib(factory=list) is certainly a step up!

attr.ib(default=new(list)) looks quite nice too, but new has a lot of baggage in Python speak.

[RonnyPfannschmidt]

for "correctness" sake attr.ib(default=attr.OciousParameterFreeDataFactory(list)) hides

[aldanor]

Don't really have a strong opinion here, but I've been finding myself really needing factories that take self lately. (I'm getting along with using __attrs_post_init__ but it's kind of a workaround...)

Wanted to also confirm this. I've run into this very often, e.g. setting one attr with other attr as a default value. Writing out

    y = attr.ib(default=attr.Factory(lambda self: self.x, takes_self=True))

is a bit too heavy though.

Wondering if takes_self could automatically be set to True if the factory argument is a callable whose signature is exactly one argument named self, as a shortcut? This introduces no ambiguity since a "normal factory" isn't expected to accept any arguments anyway. So the above could be just:

    y = attr.ib(factory=lambda self: self.x)

[aldanor]

As a working proof of concept:

from inspect import Signature, Parameter, signature

class Factory(attr.Factory):
    def __init__(self, func, takes_self=None):
        if takes_self is None:
            takes_self = signature(func) == Signature(
                [Parameter('self', Parameter.POSITIONAL_OR_KEYWORD)]
            )
        super().__init__(func, takes_self=takes_self)

def attrib(*args, factory=None, **kwargs):
    if factory is not None:
        kwargs['default'] = Factory(factory)
    return attr.ib(*args, **kwargs)

@attr.s
class Bar:
    x = attrib()
    y = attrib(factory=lambda self: self.x)

>>> Bar(42)
Bar(x=42, y=42)

[wsanchez]

@glyph: The current state of things is pretty good, yo:

@attr.s(auto_attribs=True)
class Cache(object):
    _stored: list = Factory(list)
    _by_name: dict = Factory(dict)
    _by_id: dict = Factory(dict)

[RonnyPfannschmidt]

makes me wonder if a auto_factory would be a neat thing to have

[wsanchez]

What would auto_factory mean?

Anyway, I'd assert that the above syntax addresses this ticket.

[wsanchez]

Oops didn't mean to close

[hynek]

Since this thread has already fans on Twitter and I got annoyed by the wordiness a few times myself, here’s my final offer:

attr.ib(factory=list) as syntactic sugar for attr.ib(default=attr.Factory(list, pass_self=False))

No magic, no nothing. If you need more power: there’s two other ways for it.

[asvetlov]

Sounds cool!