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
Advertise type hints with py.typed #221
Comments
A bunch of the errors were Mypy complaining about model definitions like Mypy errors:tekore\_auth\scope.py:150: error: "Type[scope]" has no attribute "read"
tekore\_auth\scope.py:163: error: "Type[scope]" has no attribute "write"
tekore\_auth\scope.py:171: error: "Type[scope]" has no attribute "every"
tekore\_auth\scope.py:171: error: "Type[scope]" has no attribute "read"
tekore\_auth\scope.py:171: error: "Type[scope]" has no attribute "write"
tekore\_model\serialise.py:44: error: Incompatible return value type (got "datetime", expected "Timestamp")
tekore\_model\local.py:59: error: Incompatible types in assignment (expression has type "str", base class "LocalItem" defined the type as "None")
tekore\_model\category.py:26: error: Incompatible types in assignment (expression has type "List[Category]", base class "Paging" defined the type as "List[Model]")
tekore\_model\category.py:26: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\category.py:26: note: Consider using "Sequence" instead, which is covariant
tekore\_model\artist.py:42: error: Incompatible types in assignment (expression has type "List[FullArtist]", base class "Paging" defined the type as "List[Model]")
tekore\_model\artist.py:42: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\artist.py:42: note: Consider using "Sequence" instead, which is covariant
tekore\_model\artist.py:54: error: Incompatible types in assignment (expression has type "List[FullArtist]", base class "Paging" defined the type as "List[Model]")
tekore\_model\artist.py:54: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\artist.py:54: note: Consider using "Sequence" instead, which is covariant
tekore\_model\show\__init__.py:25: error: Incompatible types in assignment (expression has type "List[SimpleShow]", base class "Paging" defined the type as "List[Model]")
tekore\_model\show\__init__.py:25: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\show\__init__.py:25: note: Consider using "Sequence" instead, which is covariant
tekore\_model\show\__init__.py:47: error: Incompatible types in assignment (expression has type "List[SavedShow]", base class "Paging" defined the type as "List[Model]")
tekore\_model\show\__init__.py:47: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\show\__init__.py:47: note: Consider using "Sequence" instead, which is covariant
tekore\_model\album\__init__.py:46: error: Incompatible types in assignment (expression has type "List[SimpleAlbum]", base class "Paging" defined the type as "List[Model]")
tekore\_model\album\__init__.py:46: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\album\__init__.py:46: note: Consider using "Sequence" instead, which is covariant
tekore\_model\track.py:106: error: Incompatible types in assignment (expression has type "List[FullTrack]", base class "Paging" defined the type as "List[Model]")
tekore\_model\track.py:106: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\track.py:106: note: Consider using "Sequence" instead, which is covariant
tekore\_model\track.py:116: error: Incompatible types in assignment (expression has type "List[SimpleTrack]", base class "Paging" defined the type as "List[Model]")
tekore\_model\track.py:116: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\track.py:116: note: Consider using "Sequence" instead, which is covariant
tekore\_model\track.py:146: error: Incompatible types in assignment (expression has type "List[SavedTrack]", base class "Paging" defined the type as "List[Model]")
tekore\_model\track.py:146: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\track.py:146: note: Consider using "Sequence" instead, which is covariant
tekore\_model\playlist.py:88: error: Incompatible types in assignment (expression has type "List[PlaylistTrack]", base class "Paging" defined the type as "List[Model]")
tekore\_model\playlist.py:88: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\playlist.py:88: note: Consider using "Sequence" instead, which is covariant
tekore\_model\playlist.py:146: error: Incompatible types in assignment (expression has type "List[SimplePlaylist]", base class "Paging" defined the type as "List[Model]")
tekore\_model\playlist.py:146: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\playlist.py:146: note: Consider using "Sequence" instead, which is covariant
tekore\_model\play_history.py:45: error: Incompatible types in assignment (expression has type "List[PlayHistory]", base class "Paging" defined the type as "List[Model]")
tekore\_model\play_history.py:45: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\play_history.py:45: note: Consider using "Sequence" instead, which is covariant
tekore\_model\album\full.py:56: error: Incompatible types in assignment (expression has type "List[SavedAlbum]", base class "Paging" defined the type as "List[Model]")
tekore\_model\album\full.py:56: note: "List" is invariant -- see http://mypy.readthedocs.io/en/latest/common_issues.html#variance
tekore\_model\album\full.py:56: note: Consider using "Sequence" instead, which is covariant
tekore\_client\chunked.py:7: error: The return type of a generator function should be "Generator" or one of its supertypes
tekore\_sender\concrete.py:45: error: Argument "headers" to "Response" has incompatible type "Headers"; expected "Dict[Any, Any]"
tekore\_sender\concrete.py:90: error: Argument "headers" to "Response" has incompatible type "Headers"; expected "Dict[Any, Any]"
tekore\_sender\extending.py:84: error: Missing return statement
tekore\_sender\extending.py:97: error: Item "Coroutine[None, None, Response]" of "Union[Response, Coroutine[None, None, Response]]" has no attribute "status_code"
tekore\_sender\extending.py:98: error: Item "Coroutine[None, None, Response]" of "Union[Response, Coroutine[None, None, Response]]" has no attribute "headers"
tekore\_sender\extending.py:100: error: Item "Coroutine[None, None, Response]" of "Union[Response, Coroutine[None, None, Response]]" has no attribute "status_code"
tekore\_sender\extending.py:107: error: Missing return statement
tekore\_sender\extending.py:112: error: Incompatible types in "await" (actual type "Union[Response, Coroutine[None, None, Response]]", expected type "Awaitable[Any]")
tekore\_sender\extending.py:154: error: Need type annotation for '_cache' (hint: "_cache: Dict[<type>, <type>] = ...")
tekore\_sender\extending.py:155: error: Need type annotation for '_deque'
tekore\_sender\extending.py:182: error: Value of type "Optional[Dict[Any, Any]]" is not indexable
tekore\_sender\extending.py:304: error: Item "None" of "Optional[Dict[Any, Any]]" has no attribute "update"
tekore\_sender\extending.py:307: error: Argument 2 to "_handle_fresh" of "CachingSender" has incompatible type "Union[Response, Coroutine[None, None, Response]]"; expected "Response"
tekore\_sender\extending.py:311: error: Incompatible types in "await" (actual type "Union[Response, Coroutine[None, None, Response]]", expected type "Awaitable[Any]")
tekore\_sender\extending.py:322: error: Item "None" of "Optional[Dict[Any, Any]]" has no attribute "update"
tekore\_sender\extending.py:324: error: Incompatible types in "await" (actual type "Union[Response, Coroutine[None, None, Response]]", expected type "Awaitable[Any]")
tekore\_auth\expiring\decor.py:14: error: Value of type "Optional[Dict[Any, Any]]" is not indexable
tekore\_auth\expiring\decor.py:15: error: Item "None" of "Optional[Dict[Any, Any]]" has no attribute "get"
tekore\_auth\expiring\decor.py:30: error: Argument 1 to "Token" has incompatible type "Optional[Dict[Any, Any]]"; expected "Dict[Any, Any]"
tekore\_auth\expiring\decor.py:41: error: Argument 1 to "Token" has incompatible type "Optional[Dict[Any, Any]]"; expected "Dict[Any, Any]"
tekore\_auth\expiring\client.py:78: error: Incompatible types in assignment (expression has type "None", variable has type "Dict[str, str]")
tekore\_auth\expiring\client.py:93: error: Incompatible return value type (got "Tuple[Request, Tuple[]]", expected "Token")
tekore\_auth\expiring\client.py:166: error: Incompatible return value type (got "Tuple[Request, Tuple[]]", expected "Token")
tekore\_auth\expiring\client.py:187: error: Incompatible return value type (got "Tuple[Request, Tuple[str]]", expected "Token")
tekore\_auth\expiring\client.py:261: error: Incompatible return value type (got "Tuple[Request, Tuple[]]", expected "Token")
tekore\_auth\expiring\client.py:283: error: Incompatible return value type (got "Tuple[Request, Tuple[str]]", expected "Token")
tekore\_auth\util.py:179: error: Argument 1 to "request_pkce_token" of "RefreshingCredentials" has incompatible type "Optional[str]"; expected "str"
tekore\_auth\util.py:181: error: Argument 1 to "request_user_token" of "RefreshingCredentials" has incompatible type "Optional[str]"; expected "str"
tekore\_auth\util.py:248: error: Incompatible return value type (got "Union[Token, RefreshingToken]", expected "RefreshingToken")
tekore\_auth\util.py:315: error: Incompatible return value type (got "Union[Token, RefreshingToken]", expected "RefreshingToken")
tekore\_client\decor\__init__.py:90: error: "Callable[..., Any]" has no attribute "required_scope"
tekore\_client\decor\__init__.py:91: error: "Callable[..., Any]" has no attribute "optional_scope"
tekore\_client\decor\__init__.py:92: error: "Callable[..., Any]" has no attribute "scope"
tekore\_client\decor\__init__.py:93: error: Argument 1 to "_add_doc_section" has incompatible type "Optional[str]"; expected "str"
tekore\_client\paging.py:41: error: Incompatible return value type (got "Coroutine[Any, Any, Optional[Paging]]", expected "Optional[Paging]")
tekore\_client\paging.py:50: error: Return value expected
tekore\_client\paging.py:60: error: Return value expected
tekore\_client\paging.py:77: error: Incompatible return value type (got "Coroutine[Any, Any, Optional[OffsetPaging]]", expected "Optional[OffsetPaging]")
tekore\_client\paging.py:117: error: Incompatible types in assignment (expression has type "Optional[Paging]", variable has type "Paging")
tekore\_client\paging.py:122: error: Incompatible types in assignment (expression has type "Optional[Paging]", variable has type "Paging")
tekore\_client\api\artist.py:74: error: Incompatible types in assignment (expression has type "str", variable has type "Optional[List[Union[str, AlbumGroup]]]")
tekore\_client\api\playlist\modify.py:25: error: No return value expected
tekore\_client\api\playlist\items.py:114: error: Incompatible types in assignment (expression has type "str", target has type "int")
tekore\_client\api\playlist\items.py:188: error: Need type annotation for 'gathered' (hint: "gathered: Dict[<type>, <type>] = ...")
tekore\_client\api\player\modify.py:84: error: Argument 1 to "offset_to_dict" has incompatible type "Union[int, str, None]"; expected "Union[int, str]"
tekore\_client\api\player\modify.py:116: error: Argument 1 to "offset_to_dict" has incompatible type "Union[int, str, None]"; expected "Union[int, str]"
tekore\_client\api\player\modify.py:227: error: Incompatible types in assignment (expression has type "str", variable has type "bool")
tekore\_client\api\browse\__init__.py:226: error: Incompatible types in assignment (expression has type "str", target has type "int")
tekore\_client\api\browse\__init__.py:228: error: Incompatible types in assignment (expression has type "str", target has type "int")
tekore\_client\api\browse\__init__.py:230: error: Incompatible types in assignment (expression has type "str", target has type "int")
tekore\_client\api\browse\__init__.py:232: error: Incompatible types in assignment (expression has type "str", target has type "int")
tekore\_client\full.py:67: error: The return type of a generator function should be "Generator" or one of its supertypes
tekore\_client\full.py:67: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Spotify, Any], Spotify]"; expected "Callable[..., Iterator[<nothing>]]"
tekore\_client\full.py:98: error: The return type of a generator function should be "Generator" or one of its supertypes
tekore\_client\full.py:98: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Spotify, bool], Spotify]"; expected "Callable[..., Iterator[<nothing>]]"
tekore\_client\full.py:129: error: The return type of a generator function should be "Generator" or one of its supertypes
tekore\_client\full.py:129: error: Argument 1 to "contextmanager" has incompatible type "Callable[[Spotify, bool], Spotify]"; expected "Callable[..., Iterator[<nothing>]]"
tekore\_config.py:37: error: Incompatible types in assignment (expression has type "Tuple[str, str, str, str]", variable has type "Tuple[str, str, str]")
tekore\_config.py:81: error: Argument 1 to "_read_configuration" has incompatible type "_Environ[str]"; expected "Dict[Any, Any]"
tekore\_config.py:96: error: Cannot assign to a method
tekore\_config.py:96: error: Incompatible types in assignment (expression has type "Type[str]", variable has type "Callable[[str], str]")
tekore\_config.py:148: error: Argument 1 to "_read_configuration" has incompatible type "SectionProxy"; expected "Dict[Any, Any]" |
I have an idea, maybe we can make in tekore/_client/api/album.py use real Request annotation and provide type to generic decorator: class SpotifyAlbum(SpotifyBase):
"""Album API endpoints."""
@scopes()
@send_and_process(single(), return_type=FullAlbum)
def album(
self,
album_id: str,
market: str = None
) -> Request:
"""
Get an album.
Parameters
----------
album_id
album ID
market
an ISO 3166-1 alpha-2 country code or 'from_token'
"""
return self._get('albums/' + album_id, market=market) in tekore/_client/decor/init.py make return type generic: from typing import TypeVar
ReturnType = TypeVar("RT")
def send_and_process(post_func: Callable, return_type: ReturnType) -> Callable[..., Union[ReturnType, Awaitable[ReturnType]]:
"""
Decorate a Spotify endpoint to send a request and process its content.
Parameters
----------
post_func
function to call with response JSON content and return type
"""
def parse_response(request, response):
handle_errors(request, response)
return post_func(response.content, return_type)
return _send_and_process(parse_response) in tekore/_sender/client.py expand def send_and_process(post_func: Callable) -> Callable[..., Union[Any, Awaitable[Any]]:
"""
Decorate a Client function to send a request and process its content.
The first parameter of a decorated function must be the instance (self)
of a :class:`Sender` (has :meth:`send` and :attr:`is_async`).
The decorated function must return a tuple with two items:
a :class:`Request` and a tuple with arguments to unpack to ``post_func``.
The result of ``post_func`` is returned to the caller.
Parameters
----------
post_func
function to call with the request and response
and possible additional arguments
"""
def decorator(function: Callable[..., Request]) -> Callable:
async def async_send(self, request: Request, params: tuple):
response = await self.send(request)
return post_func(request, response, *params)
@wraps(function)
def wrapper(self, *args, **kwargs):
request, params = function(self, *args, **kwargs)
if self.is_async:
return async_send(self, request, params)
response = self.send(request)
return post_func(request, response, *params)
return wrapper
return decorator in tekore/_client/process.py make def single(from_item: str = None) -> Callable:
"""
Unpack dict or items in ``from_item`` into single constructor.
If dict or ``from_item`` is None - does nothing and returns None.
"""
def post_func(json: dict, type_: type):
json = json if from_item is None else json[from_item]
return type_(**json) if json is not None else None
return post_func |
Thanks for the suggestion! It looks doable for the types that can be used in this way, but for example I realise I'm being a bit negative about all this. So just to map out the reasons a bit more: How would projects using Tekore exactly benefit from the typing advertisement? And does Mypy provide some sort of extra analysis? |
Mypy can determine whether you are using in a wrong way tekore functions or values returned by tekore API calls. Right now, for example, to use typing help in my IDE (VS Code) - I need to explicitly set type when I make an API call, otherwise it gives me either no type or wrong one: song: TYPE = await api.get_currently_playing() It is a minor inconvenience though. But I would like to help with proper typing support if you want to do it |
Oh, I thought it was automatic with other modern IDEs as well. I use PyCharm, which does provide them without For me, this could be the outline: rather than absolutely correct type hints everywhere, we should focus on providing users with the correct type hints when they make calls with Tekore. It would mean that we don't have to resolve all the issues with running Mypy on our source. I assume now that having a I'm not sure how throrough Mypy analysis is. For example, given the generic decorators you built, could the return types be correctly hinted? And if they were left as they are, to the function returning Requests, would the hints be lost in the decorators? |
@HarrySky no hurry, but I'm debating myself whether or not to release the current version with these changes. Are you available in the near future, or should we wait until the next minor with this? |
@felix-hilden sorry, saw your message, but didn't have a time to properly reply🙈 I think we can improve typing in future, with current typing annotation it is better to not include Few moments I would like to clarify - mypy is not used in VS Code for detecting types, but it doesn't detect them properly with I would like to help and improve typing, but maybe on weekend or next week, don't have a spare time for this atm |
Another idea I had - we can actually use Edit: it will take some time and tests to make it work tho |
Alright, no worries! I'll move ahead with the release soon and let's discuss this later and consider the options. |
Okay, now we could have another window of improving the hint support, since there are other changes lined up as well! From what I gather, as you mentioned, we could actually modify the So here's what should be figured out. The If you're not available in the near future @HarrySky, then this is more just a checklist for me, but I'd appreciate your comments! |
Probably will be available in December on a vacation, right now too much projects at work 😅 If I remember correctly - mypy had issues with decorators, but I don't remember exactly what the issue was. Though - example below works as should: @asynccontextmanager
async def fetcher(
schema: str = "public", deleted: bool = False
) -> AsyncIterator[Fetcher]:
[code here...]
async def test() -> None:
async with fetcher() as ft:
await ft.delete() # Fetcher has no delete() method and mypy complains here So If you need any help before December - just mention me 😄 (I also have subscribed to all tekore notifications a while ago) |
Alright no sweat! I'll try to make sense out of this a bit myself. I'm pretty out when it comes to Mypy so this is a project long overdue for me :D |
Let's once again try to advance this a bit! annot_old = wrapper.__annotations__['return']
annot_new = Union[annot_old, Coroutine[None, None, annot_old]]
wrapper.__annotations__['return'] = annot_new But, our paging endpoints are handled a bit differently, since That being said, at least PyCharm is not able to recognise the dynamically altered types and continues to mark |
@HarrySky Would you be able to try the improvements in the PR above in your IDE setup? Namely, having the runtime types corrected to be a union of sync and async variants. I had no luck with creating a script with an asynchronous function and checking it with Mypy. With or without the modification, no extra errors in the script were produced. |
@felix-hilden will try it tomorrow evening😁 |
@felix-hilden sorry for the delay, felt sick yesterday. I tested it in VSCode (I use Pyright and built-in tools for typing support) and they did not detect type of line below: token = await resources.credentials.request_user_token(code) # should be Token, but I get "Unknown" When I used
I guess that runtime manipulation with types is not detected by static analysis tools |
I am trying one approach at the moment and I saw one problem already There was a switch to internal dataclass for Request at some point and it is no longer iterable, so this line gives me errors: request, params = function(self, *args, **kwargs) # function returns Request and it can't be broken down into request and params EDIT. Feels like params is now an attribute of Request dataclass EDIT 2. Okay, I got it, it's just confusing typing :D |
Ok, I managed to make it "work". Since tekore does not separate sync/async functions - it is virtually impossible to use Union, since it will always give errors below: token = await resources.credentials.request_user_token(code)
reveal_type(token)
# mypy produces
melodiam/auth/endpoints.py:62: error: Incompatible types in "await" (actual type "Union[Token, Awaitable[Token]]", expected type "Awaitable[Any]") [misc]
melodiam/auth/endpoints.py:63: note: Revealed type is 'Any' I did not manage to make it collapse to token = await cast(Awaitable[Token], resources.credentials.request_user_token(code)) # no IDE/mypy errors
reveal_type(token) # now it is Token |
Yeah, after some research I found that there is no safe way to await if function has the same signature for sync and async and returns request_token = resources.credentials.request_user_token(code)
assert isinstance(request_token, Awaitable)
token = await request_token # no errors
reveal_type(token) # Token |
I was changing tekore files directly in virtualenv, can make a PR with my proof-of-concept code |
Yeah, I was hoping that when running Mypy on a script could at least take some advantage of runtime types, but looking back it seems like a bad idea for it to run code when type checking. What do you mean by a safe way of awaiting? Does it relate to the fact that the sync variant is not awaitable, i.e. the union isn't safe? It's really a bummer that we as programmers are absolutely certain that only one kind is returned, while analysis tools have no idea. I thought that the decorator was a fine design choice since it saved a lot of work, in loc, cognitive load and maintenance. But this would be the argument for having a separate Do you have anything else in your modifications? If not, I think a PR just for sharing code is not needed. |
By safe I mean "no errors from static checkers" :) Yeah, it's a bummer. I know that elasticsearch library, for example, has |
I think we are at a crossroads here. So here's what I'm thinking. Currently we cannot advertise the package as typed. But there should be a conversation about whether we should use a different client for async things. So I'll merge the changes (perhaps excluding the union injections), close this issue for the time being and open another one for discussing that. Then if we reach a conclusion where the second client is implemented, we can re-evaluate stricter typing. |
From #109 (@HarrySky): I think this is a broader issue than only models, so let's have it in a separate issue.
About type hints - it would be great to add py.typed file to package (PEP 561). It will show mypy that this package has type hints and it will give more helpful analysis.
tekore/py.typed
MANIFEST.in
:include tekore/py.typed
I think this could be a nice addition. However, I think our type hinting is not at the level that mypy would be happy with it. This has been discussed in #64 already, during which I decided not to change much for mypy. But maybe we ought to have another look. But I think with a
py.typed
there is a notion that the type hints are absolutely correct, which they most certainly are not at the moment. We've got most of it right, but for simplicity for example the client endpoints only return e.g.FullTrack
instead ofUnion[FullTrack, Coroutine[None, None, FullTrack]]
. I'll look into it.The text was updated successfully, but these errors were encountered: