Raise ValueError when a watcher cannot be unwatched instead of logging a warning

[maximlt]

Resolves #1003

By raising ValueError when trying to remove an already removed/never registered watcher.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.13%. Comparing base (274632a) to head (f1e252d).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1018      +/-   ##
==========================================
- Coverage   87.27%   87.13%   -0.14%     
==========================================
  Files           9        9              
  Lines        4936     4937       +1     
==========================================
- Hits         4308     4302       -6     
- Misses        628      635       +7     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[maximlt]

@jlstevens I believe you're the last person who refactored unwatch, do you mind reviewing this change :) ?

[jlstevens]

After thinking about this, I think raising a ValueError at this stage is too abrupt a change. Instead I propose the following:

1. Improve the warning to use the official warnings module instead of self_.warning
2. If we agree the ValueError is a better approach, update this warning to tell users that in future the warning will become an exception.
3. Change it to a ValueError.

I believe the original intent was to make unwatching idempotent: if you ask for something to be unwatched and it exits, it gets unwatched. If it is not being watched already, then there is nothing to do.

I see arguments either way and I don't mind this behavior being deprecated, but not this suddenly.