ericvsmith / dataclasses

Apache License 2.0
587 stars 53 forks source link

Make order=False by default? #104

Closed alanhdu closed 6 years ago

alanhdu commented 6 years ago

This is kind of a small nit, but I'm wondering whether the order parameter should be False by default instead of True? I suspect that most dataclasses don't have a natural ordering associated with them and it'd be better to throw a TypeError rather than returning at all.

Thanks for creating this library though! I'm can't wait to see this in the Python standard library!

ericvsmith commented 6 years ago

If the fields values are unorderable, you do get a TypeError:

@dataclass
class C:
    i: int
    c: complex

print(C(0, 1j) < C(0, -2j))

Traceback (most recent call last):
  File "i.py", line 44, in <module>
    print(C(0, 1j) < C(0, -2j))
  File "<string>", line 3, in __lt__
TypeError: '<' not supported between instances of 'complex' and 'complex'

Which is slightly worse that what you get with order=False:

@dataclass(order=False)
class C:
    i: int
    c: complex

print(C(0, 1j) < C(0, -2j))

Traceback (most recent call last):
  File "i.py", line 44, in <module>
    print(C(0, 1j) < C(0, -2j))
TypeError: '<' not supported between instances of 'C' and 'C'

I don't have a feel for how often instances will be ordered. But it's true that I don't often try to order attrs instances. I guess attrs doesn't have this issue because they group our eq and order together in their cmp, and you definitely want __eq__ and __ne__ by default.

My gut feeling is that we should leave order=True as the default if comparing unorderable fields raises TypeError.

If no one chimes in here with an opinion, I'll raise this on python-dev (or feel free to do that yourself).

alanhdu commented 6 years ago

Hm... I'm more worried about technically orederable field values that shouldn't be ordered (e.g. an integer ID field) than an unorderable field value like a complex number. Complex numbers are actually a good example -- they're composed of orderable fields (floats) but don't really have an order themselves.

gvanrossum commented 6 years ago

I think it should default to false, just like for regular classes (in Python 3).

ericvsmith commented 6 years ago

With this change, the default would be that you can't order classes, but when you compare them they compare by value, not identity (as is the default for regular classes). I think that's a reasonable default for something focused on the fields within classes.

gvanrossum commented 6 years ago

Agreed. No matter what the field type is it can always be compared using ==. But not everything can be ordered (and sometimes ordering behaves weird, e.g. for sets).

ericvsmith commented 6 years ago

Thanks for raising this issue, @alanhdu!