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

fix(typing): improve view type inference of ui decorators #1190

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

shiftinv
Copy link
Member

Summary

This adjusts some internal typings to get rid of Unknowns in button/select decorators.

  • view_cls.method now has a properly typed (Self@V, Item[Self@V], MsgInter) signature, instead of (Any, Unknown, MsgInter)
  • more importantly, self.method now returns an Item[Self@V] with the correctly bound view type, instead of Item[V_co], which was unbound and didn't allow accessing custom attributes on self.method.view
class CustomView(disnake.ui.View):
    x: int

    @disnake.ui.string_select(options=["A", "B"])
    async def sel(self, s, i):
        # before: StringSelect[V_co@string_select]  # <<< note, view type unbound
        # after: StringSelect[Self@CustomView]
        reveal_type(self.sel)

        # before: (Any, Unknown, MessageInteraction[Unknown]) -> Coroutine[Any, Any, Any]
        # after: (CustomView, StringSelect[Self@CustomView], MessageInteraction[Unknown]) -> Coroutine[Any, Any, Any]
        reveal_type(type(self).sel)

        # before: [error]
        # after: int
        reveal_type(self.sel.view.x)

Checklist

  • If code changes were made, then they have been tested
    • I have updated the documentation to reflect the changes
    • I have formatted the code properly by running pdm lint
    • I have type-checked the code by running pdm pyright
  • This PR fixes an issue
  • This PR adds something new (e.g. new method or parameters)
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)

- view_cls.method now has a properly typed `(Self@V, Item[Self@V], MsgInter)` signature, instead of `(Any, Unknown, MsgInter)`
- more importantly, self.method now returns an `Item[Self@V]` with the correctly bound view type, instead of `Item[V_co]`,
  which was unbound and didn't allow accessing custom attributes on `self.method.view`
@shiftinv shiftinv added t: refactor/typing/lint Refactors, typing changes and/or linting changes skip news labels May 10, 2024
@shiftinv shiftinv added this to the disnake v2.10 milestone May 10, 2024
...


T_co = TypeVar("T_co", covariant=True)
P = ParamSpec("P")


class Object(Protocol[T_co, P]):
class ItemShape(Protocol[T_co, P]):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we replace this Protocol with simply Callable[P, T_co]; I didn't realize at the time that Callable fulfills the criteria for capturing init args and return type :)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, I suppose that works too; it would widen the possible input types though, since it allows arbitrary callables and isn't necessarily limited to types. Something like Callable[P, T_co] & Type[T_co] would be cool, if we had intersection types.

Copy link
Contributor

@Enegg Enegg May 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it would widen the possible input types

I'm aware, however I don't see a reason to restrictict the accepted types. The runtime already supports that if not for the issubclass checks, this would work out of the box.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skip news t: refactor/typing/lint Refactors, typing changes and/or linting changes
Projects
Status: Todo
Development

Successfully merging this pull request may close these issues.

None yet

2 participants