Change event typing, and other fixes#4663
Change event typing, and other fixes#4663Liam-DeVoe wants to merge 11 commits intoHypothesisWorks:masterfrom
event typing, and other fixes#4663Conversation
event typingevent typing, and other fixes
| def event(value: str, payload: str | int | float = "") -> None: | ||
| def event(value: str, payload: Any = "") -> None: |
There was a problem hiding this comment.
I think this is a case where I actually prefer our type annotations to be stricter than the runtime logic - while more exotic payloads 'mostly work', it'd be nice to keep weird problems with string conversion as the user's problem.
For the same usage-hint reason that we say value: str, let's keep payload: str | int | float.
There was a problem hiding this comment.
I disagree with this. My interpretation of your position: type hints have two purposes; documentation, and correctness. We type payload: str | int | float to communicate "these are the normal types you'll be passing" to a user seeing event for the first time. But this comes at the expense of requiring other users to add type: ignore at their usage sites. Since we explicitly document that string-convertible objects are valid, I think the types should reflect this.
What about payload: SupportsStr, which is a typing protocol that requires __str__? I hadn't realized we do the same conversion for value, so I'd also support value: SupportsStr.
event(value, payload)accepts any string-coercible value inpayload, and the type hint ofAnynow reflects this.ModuleTypeand cleaner otherwise.b33e473 is the result of
FORMAT_ALL=true ./build.sh format. It's a bug that we didn't format all files at tool bump, but I don't have a root cause narrowed down.