Fix hashing behavior

[hynek]

Our previous default behavior was incorrect. Adding __hash__ has to be deliberate action and only makes sense for immutable classes.

Also set the doc theme to alabaster because the RTD one looks weird if a parameter doc has multiple paragraphs.

cc @njsmith @glyph @Tinche

[codecov-io]

Codecov Report

Merging #142 into master will not change coverage.
The diff coverage is 100%.

@@          Coverage Diff          @@
##           master   #142   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           8      8           
  Lines         544    551    +7     
  Branches      119    122    +3     
=====================================
+ Hits          544    551    +7

Impacted Files	Coverage Δ
src/attr/init.py	100% <ø> (ø)	✅
src/attr/_make.py	100% <100%> (ø)	✅

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0563f44...ef30deb. Read the comment docs.

[Tinche]

I'm in favor. I think this will be a clear improvement for 95% of the usage out there.

I think we could make the behavior smarter in the default case but honestly I don't know if it's worth the complexity. For example, we can detect if a custom __hash__ method has been written on the class we're decorating and if so, default to doing nothing even if @attr.s(hash=None). (Or raise an exception and ask the user to clarify.)
Also an idea floated around was to generate __hash__ for frozen classes by default. But I've used __hash__ so rarely I don't really have a strong opinion.

[hynek]

Trying to be smarter tends to be recipe for future disaster in my experience. :)

I think we should offer explicit APIs for certain archetypes but not try to guess what people want when they set certain flags…

[glyph]

An observation about compatibility: if the attr.ib list included a a mutable object (say, a dict or a list) is present in the list of hashed attributes, you'd already get an exception.

If we don't add a __hash__, that takes this to a silent id-based success, but this in fact adds a __hash__, just one that raises an exception.

This is "incompatible" in that code which relies on hash=True and is implicitly immutable will stop working, but the fix is pretty simple.

[hynek]

I have updated it according to @njsmith & @ambv ’s wishes.

The reason why I changed my mind is that if we break b/w compatibility to fix bad behavior, we should do it properly and make it worth it.

Please feed all the back.

[hynek]

cough Nathaniel?

[njsmith]

It looks like the logic that's implemented here is actually different from my suggestion? Specifically, in the case where if cmp=False (i.e., we're inheriting the superclass __eq__, if any), then I think we should also act like hash=False (i.e., also inherit the superclass __hash__, if any). Is that intentional, or...?

[hynek]

No, I just happen too be too dumb to program a computer properly. :(

It would be kind if you could have another look…

[njsmith]

I can't say I've read through every line with careful attention, but LGTM

[hynek]

Thank you for your patience. ❤️

[njsmith]

Thank you for all your work!