-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add stubs for WebOb #9874
Add stubs for WebOb #9874
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Is this PR ready for proper review? If so, please take it out of draft :) The annotations certainly don't need to be complete for this to be merged; we encourage incremental submissions of annotations! It makes it much easier for us to review. |
I'd like to play around with the annotations in a couple of different code bases first to see if there are any particularly rough edges or stuff that doesn't resolve as intended. I won't really have time to do that until later this week though. |
Take your time, there's no rush! Was just clarifying the status :) |
Since mypy doesn't support asymmetric properties yet, I've had to switch to a custom descriptor implementation for those properties.
This comment has been minimized.
This comment has been minimized.
I did some testing in various codebases and in a sandbox with All in all I'm satisfied from a library users' perspective now, so this should be ready for a more general review. |
The header property setters previously all accepted `bytes` as an input for the header, but upstream enforces the use of `str`, so it's better to be strict, so people's code won't pass type checking if it will break in the next version of WebOb.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Also fixes small mistake in `Cookie.load` and `Cookie.__init__`
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@overload | ||
def __set__(self, __obj: Any, __value: str | None) -> None: ... | ||
@overload | ||
def __set__(self, __obj: Any, __value: AcceptNoHeader | AcceptValidHeader | AcceptInvalidHeader) -> None: ... | ||
@overload | ||
def __set__(self, __obj: Any, __value: SupportsItems[str, float | tuple[float, str]]) -> None: ... | ||
@overload | ||
def __set__(self, __obj: Any, __value: _ListOrTuple[str | tuple[str, float, str] | list[Any]]) -> None: ... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Doesn't seem like this needs overloads, you can just use a union as the argument type for __value
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are completely correct, however in this case I think a bunch of overloads reads more nicely than a very large type union. I've seen overloads used like this in other places in typeshed and liked how it looks compared to a very large type union where after maybe the fourth or fifth type in the union you just stop reading, especially once the types get as complicated as the last two overloads.
So here it's separated neatly into the plain string version of the header (as per the HTTP spec), then the specialized objects for representing the header from WebOb, then the Mapping like way to represent this data, and then the sequence like way to represent it.
You could make a case for combining the first two overloads, but I think the other two should stay distinct. I've also debated whether or not to add the final overload at all, because it isn't completely type safe and kind of confusing to read, it just happens to be one of the supported and documented ways to pass this data in.
Fixes a couple of typos in the `stubtest_allowlist.txt` comments Co-authored-by: Jelle Zijlstra <[email protected]>
This comment has been minimized.
This comment has been minimized.
Implements bare minimum on `webob.dec.wsgify` to make the decorator work
This comment has been minimized.
This comment has been minimized.
Fixes annotation of `dicts` and `copy` on `NestedMultiDict`
This comment has been minimized.
This comment has been minimized.
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
I've already created some more simple private stubs for WebOb, so I thought I'd try to clean them up and contribute them to the community. WebOb makes very heavy use of descriptors, which I've tried to fully leverage, but I will have to test if all the overloads actually work correctly. It might be worth going back to something more simplistic again which replaces some of the descriptors which just plain attributes, even if WebOb internally would be more generous what it accepts as an input for the assignment.