Don't use hash to avoid attrs warning

[CoolCat467]

Fixes #3053, this pull request replaces all uses of @attrs.define(... hash=x) with @attrs.define(... unsafe_hash=x).
In attrs >= 24.1.0, a warning is raised that hash is going to be removed in August of 2025.
unsafe_hash is an alias for hash as of attrs v22.2.0, so there should be no functional changes.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.63%. Comparing base (20f9291) to head (ea68af7).
Report is 176 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3054   +/-   ##
=======================================
  Coverage   99.63%   99.63%           
=======================================
  Files         120      120           
  Lines       17832    17832           
  Branches     3204     3204           
=======================================
  Hits        17767    17767           
  Misses         46       46           
  Partials       19       19           

Files with missing lines	Coverage Δ
src/trio/_core/_entry_queue.py	100.00% <100.00%> (ø)
src/trio/_core/_io_epoll.py	100.00% <100.00%> (ø)
src/trio/_core/_local.py	100.00% <100.00%> (ø)
src/trio/_core/_parking_lot.py	100.00% <100.00%> (ø)
src/trio/_core/_run.py	99.38% <100.00%> (ø)
src/trio/_core/_tests/test_instrumentation.py	100.00% <100.00%> (ø)
src/trio/_highlevel_generic.py	100.00% <100.00%> (ø)
src/trio/_sync.py	100.00% <100.00%> (ø)
src/trio/_tests/test_highlevel_serve_listeners.py	100.00% <100.00%> (ø)
src/trio/testing/_sequencer.py	100.00% <100.00%> (ø)

[richardsheridan]

I'm sure none of our code actually needs this argument, we should be using just eq=false or frozen=true to get the behavior we need. These were all put in before attrs was giving clear guidance on the feature.

Surely the tests all pass if you just remove the hash=false text?

[CoolCat467]

Surely the tests all pass if you just remove the hash=false text?

No idea, haven't tested that, but I was trying to avoid changing functionality.

[A5rocks]

Yeah given attrs says:

The overarching theme is to never set the @attrs.define(unsafe_hash=X) parameter yourself. Leave it at None which means that attrs will do the right thing for you, depending on the other parameters:

it's probably a good idea to not use the parameter.

---

And it looks like eq=False implies unsafe_hash=False:

https://github.com/python-attrs/attrs/blob/6d0e01f5e4e68a2eda5a5c2b826aa6961062be52/src/attr/_make.py#L1368

[A5rocks]

In addition to removing all unsafe_hash=Falses, can you add a newsfragment? You added a tag not to, but I think it's a good thing to talk about for people just reading the changelogs.

EDIT: also, could you bump attrs in the locked test requirements so we can be sure there's no other warnings?

[CoolCat467]

Looks good @A5rocks

[A5rocks]

Let's let others check this, I'll only make a release tomorrow anyways.

[CoolCat467]

Yea mostly commenting on the additional changes you made

[agronholm]

LGTM.

[agronholm]

I need this for AnyIO to pass its test suite on 3.13 😃

[hynek]

Whoa, y'all are fast! I ran into the warning in stamina and wanted to reach out to you how obnoxious it is for you and if I should just soft-deprecate it instead.

Looks like you got most of them except (at least)

trio/src/trio/_threads.py

Line 142 in 223e4f4

@attrs.frozen(eq=False, hash=False, slots=False)

A fun fact on the side: attrs's guidance on hashing goes entirely back to @njsmith opening my eyes and causing a big rewrite of that feature within attrs. So the chances are excellent that you don't need the argument at all.

Tangentially, unsafe_hash has been added to attrs already in 22.2.0, so if you want, you can relax that lower pin a bit.

[A5rocks]

I ran into the warning in stamina and wanted to reach out to you how obnoxious it is for you and if I should just soft-deprecate it instead.

We've already handled it and it's been fine: it revealed two configuration issues so that's nice! (one is that deprecation warnings get ignored if the module is imported as a plugin first, the other is that one of the CI runs should use test-requirements.txt as a constraints file but doesn't)

[CoolCat467]

Looks like you got most of them except (at least)

trio/src/trio/_threads.py

Line 142 in 223e4f4

@attrs.frozen(eq=False, hash=False, slots=False)

Unfortunate, surprised CI didn't catch that, but this is because when I made this pull request I did git grep attrs.define | grep hash if I remember correctly, so will need to do another pass looking for anything not using attrs.define

[hynek]

I did grepped later for hash= and found only this one instance so I think you’ll be fine!