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

clean division between "value" and "object" #3

Closed
glyph opened this issue Jan 29, 2015 · 21 comments · Fixed by #60
Closed

clean division between "value" and "object" #3

glyph opened this issue Jan 29, 2015 · 21 comments · Fixed by #60
Milestone

Comments

@glyph
Copy link
Contributor

glyph commented Jan 29, 2015

Objects, as in object-oriented programming, are side-effecty, mutable state, whose methods ought to represent I/O.

Values, as in functional programming, are immutable data, whose methods ought to represent computation.

Python does not have as good a division between these very different sorts of beasts as it should, but it does have one critical distinction: the __hash__ method.

characteristic has this issue which crops up occasionally where you end up with objects that you can't put into a dictionary as a key, because one of its attributes is a dictionary. Sometimes people expect to be able to do this because characteristic makes so many other things easy, and just expect dictionaries to suddenly be immutable; sometimes people expect hash-by-identity.

I propose that attr provide a way to specifically create two types of objects with a distinct interface; one that creates a "value" and one that creates an "object", so that users can see issues around mutability far earlier in the process.

So, for example, the "object" type would:

  • not provide __hash__ by default, provide an __eq__ that does structural equality, and __gt__/__lt__ that just raise exceptions
  • if asked, provide an identity-based __hash__, but then also switch to an identity-based __eq__

and the 'value" type would

  • call hash() on all of its arguments at construction time so it would fail immediately if it contained a mutable type
  • fail immediately at class-definition time if any validator is mutable
  • provide immutable descriptors for all its attributes
@hynek
Copy link
Member

hynek commented Jan 29, 2015

How would that look API-wise? Would something like @attr.object or @attr.value be presets for @attr.s?

@glyph
Copy link
Contributor Author

glyph commented Jan 29, 2015

I'm not sure. I'm trying to decide what I think the default for attr.s is; there's a strong case in my mind for both default-to-mutable or default-to-immutable. attr.s(mutable=True)?

@hynek
Copy link
Member

hynek commented Jan 29, 2015

The default should be what most people (and I!) would expect it to do. We’re still on Python and don’t have free COW structures etc. Therefore any immutability gimmickry ought to be opt-in (but in place and simple to use).

@glyph
Copy link
Contributor Author

glyph commented Jan 30, 2015

I think you're right; Python programmers are going to expect mutability by default, trying to turn that off would just be an ideological statement, not useful functionality. So how about just having a __hash__ that raises an exception which points you at attr.value, and attr.s is object?

@radix
Copy link
Contributor

radix commented Jun 2, 2015

Hi, I found this ticket because I was looking for an immutable argument to @attr.s like there was with characteristic :)

ALSO: on the subject of "value types", I would like to point out my new library sumtypes (honestly I didn't come here looking for a place to advertise, but it seems relevant). immutable would be great to have for it too (and I probably would make it the default in that library).

@hynek
Copy link
Member

hynek commented Jun 3, 2015

I’m reluctant to implement immutable myself because it adds another method I have to highly invasively muck with (__setattr__).

But attrs is extensible, so feel free to implement it yourself!

@radix
Copy link
Contributor

radix commented Jun 3, 2015

@hynek I guess there's two separate aspects of immutability: making sure declared attributes are immutable, and making sure new attributes can't be dynamically assigned to.

Neither of these actually require defining __setattr__. The former can be accomplished with the descriptor protocol, and the latter can be accomplished with __slots__.

I assumed Attribute must have already implemented the descriptor protocol, but I see now I'm wrong, and, for example, mutating an attribute post-instantiation will allow setting it to a value that a validator wouldn't accept, because validators are not run on mutation. So maybe it would be best if Attribute instances do provide the descriptor protocol, both to support validating mutation as well as implementing immutability if it's requested?

@glyph
Copy link
Contributor Author

glyph commented Jun 4, 2015

I kinda want to recommend setting up a performance test suite at this point, because while it seems to me like using the descriptor protocol ought to be zero-cost on PyPy and "fast enough" on CPython, it also seems like it would be worth knowing that for sure.

(Although I also assumed that attrs would validate on mutation and this is a slightly unpleasant surprise. Hmm.)

@hynek
Copy link
Member

hynek commented Jun 4, 2015

attrs had validation on mutation once but there were so many loopholes that I decided to take it out because the performance hit for everyone simply wasn’t worth it.

I’m pretty sure that using descriptors is measurably slower. I’ll happily be proven wrong with a benchmark. :)

@glyph
Copy link
Contributor Author

glyph commented Jun 4, 2015

What do you mean "so many loopholes"? Other than __dict__.__setitem__ and object.__setattr__ what else is there?

@radix
Copy link
Contributor

radix commented Jun 5, 2015

@glyph well, there's also other methods on __dict__ than __setitem__, so generalize that to "__dict__ mutation"

Personally I think it's worth doing validation with the descriptor protocol even though someone could bypass with __dict__ mutation. However! It would also be possible to make naive dict mutation ineffective, since Python always consults a descriptor before a __dict__ entry. It might have other downsides, but you could have the descriptors store their actual data under a different name in the instance dict, so at least someone would really know they're bypassing validation when they're assigning something to __dict__['__attrs_x'].

But... yeah, that might be a bit too surprising/weird for people who do want to dig down into the representation. I mean, Pickling would still work just fine, but maybe there are some useful things that would break?

@radix
Copy link
Contributor

radix commented Jun 5, 2015

Also, as a follow-up to my previous mention of __slots__, I've hit a snag there: apparently the way __slots__ works is by creating descriptors on the class. Which means you can't use your own descriptors (or even non-descriptors -- you can't even use class variables to declare defaults for instances, for example). The result of trying to add __slots__ to a @attrib.s-using class is this:

>>> @attr.s
... class Foo(object):
...  __slots__ = ('x', 'y')
...  x = attr.ib()
...  y = attr.ib()
...
>>> Foo(1, 2)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "<attrs generated init a7439b12cdcdd2d603c04767f9a798adb8a0d944>", line 2, in __init__
AttributeError: 'Foo' object attribute 'x' is read-only

There may be some clever trick I'm missing to still use __slots__ (which actually would be nice to use on CPython if we could...), but I'm thinking @hynek is right and the only way to implement immutability would be by defining a __setattr__ :-(

@radix
Copy link
Contributor

radix commented Jun 5, 2015

Oh, never mind. There is a way to use both slots and descriptors:

http://stackoverflow.com/questions/4912499/using-python-descriptors-with-slots

It's just a matter of having the user's descriptor be named different from the slot.

@hynek
Copy link
Member

hynek commented Jun 7, 2015

most obvious loophole: instance.attribute.do_something_that_mutates_instance().

I don’t want to sound like an ass but adding features that are invasive but only interesting to a small fraction of people made characteristic what it is so I’m much more conservative this time and rather made it extensible than implementing everything ppl shout at me.

I’m open to descriptor-based solutions that have no negative impact on performance though.

@glyph
Copy link
Contributor Author

glyph commented Jun 7, 2015

Mutability doesn't seem like a particularly obscure or minority concern :).

That said, I'm not sure what you're objecting to. Doing validation on attributes by default? Doing validation on attributes at all? Or the actual topic of this bug, @attr.value, which would not be on by default anyway?

@hynek
Copy link
Member

hynek commented Jun 7, 2015

Echo chamber. :)

I’m objecting the tangent of validating on assignment. I don’t object immutability but I feel it should be a separate decorator. Maybe @attr.value? :)

@glyph
Copy link
Contributor Author

glyph commented Jun 7, 2015

OK good. Let's have another issue to discuss that.

For the case of @attr.value specifically, we could just override __setattr__ to raise AttributeError after initialization. Does that seem sufficient?

@glyph
Copy link
Contributor Author

glyph commented Jun 7, 2015

You mentioned not wanting to do it yourself, but nothing except value would need to mess with it.

@hynek
Copy link
Member

hynek commented Jun 7, 2015

Just to be clear: my main issue is not being lazy but wanting to keep attrs clean. :) Moving settattr magic into a separate decorator seems a fair compromise to me.

@glyph
Copy link
Contributor Author

glyph commented Jun 7, 2015

Cool, thank you, I asked the question very awkwardly but that was exactly the form of answer I wanted :).

@glyph
Copy link
Contributor Author

glyph commented Aug 15, 2016

See also #50 ?

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 a pull request may close this issue.

3 participants