Complete error signature of _InValidator

[iron3oxide]

Summary

This makes the _InValidator produce the error signature it is proclaimed to have in the docstring of _in(). Similarly to the other validators, the attribute, conflicting factor (in this case self.options) and the value are now included when a ValueError is raised.

Pull Request Check List

• Added tests for changed code.
  Our CI fails if coverage is not 100%.
• New features have been added to our Hypothesis testing strategy.
• Changes or additions to public APIs are reflected in our type stubs (files ending in .pyi).
  • ...and used in the stub test file tests/typing_example.py.
  • If they've been added to attr/__init__.pyi, they've also been re-imported in attrs/__init__.pyi.
• Updated documentation for changed code.
  • New functions/classes have to be added to docs/api.rst by hand.
  • Changes to the signature of @attr.s() have to be added by hand too.
  • Changed/added classes/methods/functions have appropriate versionadded, versionchanged, or deprecated directives.
    Find the appropriate next version in our __init__.py file.
• Documentation in .rst files is written using semantic newlines.
• Changes (and possible deprecations) have news fragments in changelog.d.
• Consider granting push permissions to the PR branch, so maintainers can fix minor issues themselves without pestering you.

I hope it's ok that I checked all the boxes that don't apply as well (e.g. sufficient documentation is already there).

Notes

It may be advisable to only include a gist of the options because otherwise this change could negatively impact performance if _in() is called on a large collection (the error message would have to use that gist as well of course).

My initial goal though was to make the ValueError do what the docstring says it does. One may want to catch a ValueError and present the user with a more "polished" (or in my case also translated) version of it. A standardized error signature can help with that, that's why _in() would now produce the same signature as e.g. matches_re() would. I think this was also the intention of the person that wrote the _in() docstring. Hope this is helpful, it's my first PR ever!

[iron3oxide]

Looks like an InternalServerError prevented the 3.9 tests from succeeding..

[hynek]

I’m afraid that would be a backwards-incompatible change. I also suspect the docs was a copy/paste mistake and the docs should be adapted to the behavior and not vice versa?

[iron3oxide]

In theory, adapting the docs to the actual behavior is the way to go, no question about it. In this case, I thought the docstring expressed the intended behavior and the method just never got "completed".

What about adding a default argument like verbose_value_error=False to _in() and passing it to _InValidator? That way, the change should be backwards-compatible but people that need a more verbose error message could obtain it.

[hynek]

Agh ugh…that seems a lot of complexity for something that almost nobody would notice.

You know what? Let's treat it as a bug and fix it to conform to the documented behavior.

Make the newsfragment breaking and please use semantic newlines.

[iron3oxide]

Semantic newlines seem pretty neat, thanks for making me discover them! I actually realized that one can also obtain the values in question from the caught error, so I won't insist on merging this if the complexity it adds outweighs the benefits in your eyes.

[hynek]

What I meant is that I'm willing to accept your original approach of adapting the behavior to the documentation since I don't expect a lot of breakage to make this complex/special case.

JFTR you've gone a bit overboard with semantic newlines…I'd call that "Poetic Newlines" 😅. We usually just add newlines after periods, colons and sometimes semicolons. :) (comma is OK, but middle of the sentence is not)

[iron3oxide]

That's probably a sane strategy for semantic newlines, I can see why breaks anywhere else might seem arbitrary. I tried to follow the style used in the article you linked, found here, where breaks in the middle of a sentence can occur at "breaks between clauses", but finding these breaks can of course be cumbersome, and deciding if a break occurs may even be subjective in some cases.

Either way, I didn't quite understand the imperative of your reply, or to put it bluntly: what should I do next?

[hynek]

• Roll back to your original solution of the validator behaving like it is currently documented.
• Reduce new lines to whole sentences.

[iron3oxide]

Lessons learned the hard way: git reset does not work how I thought it would (never needed to use it before) and manually reverting changes is in most cases better than using git revert. Sorry for the mess.

[hynek]

I'll refine the changelog a bit on my own since I've pestered you enough, so thanks for playing along for this long!