[pycurl] Fill in missing/Incomplete annotations#15919
Open
Psychosoc1al wants to merge 3 commits into
Open
Conversation
This comment has been minimized.
This comment has been minimized.
Contributor
|
Diff from mypy_primer, showing the effect of this PR on open source code: tornado (https://github.com/tornadoweb/tornado)
+ tornado/curl_httpclient.py:55: error: Argument 2 to "setopt" of "CurlMulti" has incompatible type "Callable[[int, int, Any, bytes], None]"; expected "bool | int | list[str | bytes] | tuple[str | bytes, ...] | Callable[[int], Literal[-1, 0] | None] | Callable[[int, int, CurlMulti, Any | None], Literal[-1, 0] | None] | None" [arg-type]
|
Contributor
Author
|
The first error mentioned in #15919 (comment) was indeed a bug on my end: I had mixed up the unsupported However, the failure in #15919 (comment) appears to be a downstream issue. It stems from a loosely annotated, unused How should I proceed with this conflict? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR fills in missing/incomplete type annotations for
pycurlpackage.Reasoning
Missing annotations:
Curl.setopt()(docs, source code): Since this method accepts a wide variety of types, I was a bit scared of a huge annotation. It would include all kinds of objects, ranging fromint,strand file-like objects to multiple signatures ofCallableand something like[(str | bytes, (str, bytes, (int, str | bytes)))](for legacy, but supportedHTTPPOST). For now, I have annotated it withAny | None. I am open to suggestions on how to structure this more strictly.CurlMulti.setopt()(docs, source code): This method is much more restricted than previous one. I went through documentation and source code and did a small brute-force over all available 12 options for this method to ensure.CurlShare.setopt()(docs, source code): Similar to the above, but only limited to two options that only acceptint. Also ensured with a simple brute-force check.CurlMulti.assign()(docs, source code): Thevalueparameter isAny | Noneby design.Noneis used to unassign, and other Python objectscan be passed to store their reference so they can be retrieved later in a callback without any processing by curl itself (works roughly like this: store "socket: object" relation -> *something happens on that socket* -> callback called -> object related to that socket is passed to callback).Incompleteannotations:version_info()(docs, source code): TheseIncompletefields use the same underlying string conversion function (vi_str()) as other string-returning fields in the structure. However, these two specific values can also beNone, which seems to match libcurl documentation.CurlMulti.perform()(docs, source code): The "status" returned is just anint, similar to other functions: libcurl docs.CurlMulti.fdset()(docs, source code): The C-level file descriptor sets (fd_set) are translated to Pythonlist[int], here source code is pretty straightforward.CurlMulti.info_read()(docs, source code): The return structure is a bit more complex, but described in details in documentation