Added annotation for five functions in Authlib#14800
Added annotation for five functions in Authlib#14800Spider84pr wants to merge 41 commits intopython:mainfrom
Conversation
This comment has been minimized.
This comment has been minimized.
|
I am especially doubting if i must import Request? |
|
I have look in existing stubs and I found how to do it. Hopefully it is correct. |
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.
|
Some tests failed I have googled and added to Response import # type: ignore[import-untyped] |
| from logging import Logger | ||
| from typing import Any | ||
|
|
||
| import requests # type: ignore[import-untyped] |
There was a problem hiding this comment.
if you are importing a 3rd-party lib, you need to add types-requests as a dependency: https://github.com/python/typeshed/blob/main/stubs/Authlib/METADATA.toml
There was a problem hiding this comment.
@sobolevn I must do it like so?
Metadata. toml
dependencies = [
"types-httpx",
]
Sorry, I've faced it first time.
There was a problem hiding this comment.
@Spider84pr requires = ["types-requests"] should be correct. See for example stubs/pysftp/METADATA.toml.
| async def request(self, method, url, token=None, **kwargs): ... | ||
| async def create_authorization_url(self, redirect_uri=None, **kwargs): ... | ||
| async def fetch_access_token(self, request_token=None, **kwargs): ... | ||
| async def request(self, method: str, url: str, token: dict[str, Any] | None = None, **kwargs) -> requests.Response: ... |
There was a problem hiding this comment.
Are you sure that this is not httpx.Response? requests is not an async lib.
There was a problem hiding this comment.
Yes, I've made mistake - will fix it
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| from logging import Logger | ||
| from typing import Any | ||
|
|
||
| import httpx |
There was a problem hiding this comment.
Please, add httpx to https://github.com/python/typeshed/blob/main/stubs/Authlib/METADATA.toml#L3
| async def create_authorization_url(self, redirect_uri=None, **kwargs): ... | ||
| async def fetch_access_token(self, redirect_uri=None, **kwargs): ... | ||
| async def load_server_metadata(self) -> dict[str, Any]: ... | ||
| async def request(self, method: str, url: str, token: dict[str, Any] | None = None, **kwargs) -> httpx.Response: ... |
There was a problem hiding this comment.
Don't forget to type **kwargs if your touching this function. The same applies for all others.
Co-authored-by: sobolevn <mail@sobolevn.me>
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.
This comment has been minimized.
This comment has been minimized.
|
Hello. Authlib requires cryptography for work. I have installed if on my local machine and all tests runs OK. I have googled and it reccomended to put it in /requirements-tests.txt Tried to do it in PR - this test are passed but appeared a lot of other error in other stub files. I thought that it was bad idea and removed it back from requirements-tests. As previously mentioned tests found many errors in other files. After this PR I can try work, on it. |
|
Just add new errors to https://github.com/python/typeshed/blob/main/stubs/Authlib/%40tests/stubtest_allowlist.txt with a @srittau can you please help with the |
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. 🤖🎉 |
|
@sobolevn I've added entries in stubtest_allowlist.txt and I will work it in next PR but I cannot resolve the remaining two test. Must I just wait? |
Co-authored-by: sobolevn <mail@sobolevn.me>
|
@sobolevn thanks for corrected me.Also I have succesfully setup stub_ uploader on local machine. Hopefully it will help me in future PRs. |
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
@sobolevn now only one test remains failing. It is very strange because your commit is merged several days ago. I have looked in source of this test - it trying to find requires_dist in the Authlib sources not in stubs that we can edit. |
|
Maybe I need to to a PR to Authlib to as them to add this dependency to metadata? |
|
@Spider84pr Hello and thank you for PR. Could you merge main branch and resolve conflicts? Everything looks ready to pass CI now |
5 participants
I have added five annotation in Authlib
Not sure how to annotate returning request function of AsyncOAuth1Mixin class
It seem returning Response object from request library. I am not sure how to annotate it correctly
Can I write it like this?
async def request(self, method, url: str, token: dict[str, Any] | None =None, **kwargs): -> Response ...