Skip to content
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

Closed
felix-hilden opened this issue Oct 17, 2020 · 23 comments
Closed

Advertise type hints with py.typed #221

felix-hilden opened this issue Oct 17, 2020 · 23 comments
Labels
enhancement New feature or improvement wontfix This will not be worked on

Comments

@felix-hilden
Copy link
Owner

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.

  • Create empty file tekore/py.typed
  • Add line to 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 of Union[FullTrack, Coroutine[None, None, FullTrack]]. I'll look into it.

@felix-hilden felix-hilden added enhancement New feature or improvement consideration Future decision to be discussed labels Oct 17, 2020
@felix-hilden
Copy link
Owner Author

felix-hilden commented Oct 19, 2020

A bunch of the errors were Mypy complaining about model definitions like field: int = None, which were easy to fix. However, many endpoints have much deeper issues. For example, we always return a request from endpoints for internal processing, but type hint the actual returned model type. This is naturally beyond any type checkers. I'll include the full Mypy log below for reference. Line numbers will naturally change. Looking at them with knowledge of the internals, they seem correct for Mypy to complain about but wrong for us to fix. So as it is currently, I see no practical way of declaring Tekore as fully (or correctly) typed. Thoughts, @HarrySky?

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]"

@HarrySky
Copy link
Contributor

I have an idea, maybe we can make @send_and_process generic like this:

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 Callable return type:

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 single() returned function accept type:

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

@felix-hilden
Copy link
Owner Author

Thanks for the suggestion! It looks doable for the types that can be used in this way, but for example Tuple[str, PlaylistPaging], or whatever it is in featured playlists if I remember correctly, it would be harder. And it's not just a matter of getting the types to parse, because my main reason for using them is IDE code completion. I'm not sure they will be able to figure out the type that's being returned given the Request hints.

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?

@HarrySky
Copy link
Contributor

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

@felix-hilden
Copy link
Owner Author

felix-hilden commented Oct 21, 2020

Oh, I thought it was automatic with other modern IDEs as well. I use PyCharm, which does provide them without py.typed. But if other IDEs use Mypy to provide that, it makes sense. In that case I would consider this a much bigger priority. I'd appreciate your help a lot! So let's start planning.

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 py.typed does not mean passing Mypy with default parameters. But this would mean that for example endpoints should use the Union[Type, Coroutine[None,None,Type]] hints. Thoughts?

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?

@felix-hilden
Copy link
Owner Author

@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?

@HarrySky
Copy link
Contributor

@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 py.typed for now.

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 await in front of API call.

I would like to help and improve typing, but maybe on weekend or next week, don't have a spare time for this atm

@HarrySky
Copy link
Contributor

HarrySky commented Oct 22, 2020

Another idea I had - we can actually use send_and_process inside function and make function annotation right.

Edit: it will take some time and tests to make it work tho

@felix-hilden
Copy link
Owner Author

Alright, no worries! I'll move ahead with the release soon and let's discuss this later and consider the options.

@felix-hilden
Copy link
Owner Author

felix-hilden commented Nov 24, 2020

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 send_and_process in tekore/_sender/client to inject the awaitable union type hint into each endpoint. Was this what you meant? It could very well be the first (and most important) step.

So here's what should be figured out. The function argument includes the type hint, after which the wrapper takes over. Firstly: does Mypy infer type hints from source code or runtime attributes? If source code, I think we're in deep, but I doubt it is the case. If runtime, then it's doable like you mentioned. Then we would need to figure out does functools.wraps propagate the type hints. If not, the hints need to be manually propagated through all our decorator layers. Anything else I'm missing?

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!

@HarrySky
Copy link
Contributor

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 @asynccontextmanager and fetcher() have different annotations, but mypy checks decorator one, same can be done for @send_and_process + get_currently_playing(), etc.

If you need any help before December - just mention me 😄 (I also have subscribed to all tekore notifications a while ago)

@felix-hilden
Copy link
Owner Author

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

@felix-hilden
Copy link
Owner Author

felix-hilden commented Jan 10, 2021

Let's once again try to advance this a bit! wraps does indeed propagate type hints forward, and they are available in Spotify.endpoint.__annotations__. So we could do this in client/send_and_process:

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 send_and_process is only used internally, not in the top endpoints. So we would have to manually change the type hints and disable the automatic when no return types are found or alternatively find a better way of doing paging.

That being said, at least PyCharm is not able to recognise the dynamically altered types and continues to mark await spotify.track as invalid. That shouldn't be a reason for us to not try to get the runtime type hints correct, but it is a bit disappointing. Maybe the generic decorator approach would work. I'll have to check.

@felix-hilden felix-hilden mentioned this issue Jan 12, 2021
3 tasks
@felix-hilden
Copy link
Owner Author

@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.

@HarrySky
Copy link
Contributor

@felix-hilden will try it tomorrow evening😁

@HarrySky
Copy link
Contributor

@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 reveal_type(token) mypy reports this:

melodiam/auth/endpoints.py:63: note: Revealed type is 'Any' 

I guess that runtime manipulation with types is not detected by static analysis tools

@HarrySky
Copy link
Contributor

HarrySky commented Jan 14, 2021

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

@HarrySky
Copy link
Contributor

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 Awaitable[Token] when using await and since await expects only Awaitable - I need to use a mypy.cast for it to work:

token = await cast(Awaitable[Token], resources.credentials.request_user_token(code))  # no IDE/mypy errors
reveal_type(token)  # now it is Token

@HarrySky
Copy link
Contributor

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 Union. I can only narrow the type after the fact:

request_token = resources.credentials.request_user_token(code)
assert isinstance(request_token, Awaitable)
token = await request_token  # no errors
reveal_type(token)  # Token

@HarrySky
Copy link
Contributor

I was changing tekore files directly in virtualenv, can make a PR with my proof-of-concept code

@felix-hilden
Copy link
Owner Author

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 AsyncSpotify or something like that... Those casts and such would really be off-putting to write into real code, at least in my opinion.

Do you have anything else in your modifications? If not, I think a PR just for sharing code is not needed.

@HarrySky
Copy link
Contributor

HarrySky commented Jan 14, 2021

By safe I mean "no errors from static checkers" :) Yeah, it's a bummer. I know that elasticsearch library, for example, has AsyncElasticsearch with the same API as in Elasticsearch class just for async/await usage.

@felix-hilden
Copy link
Owner Author

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.

@felix-hilden felix-hilden added wontfix This will not be worked on and removed consideration Future decision to be discussed labels Jan 15, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or improvement wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants