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.

Sign up for GitHub

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

Jump to bottom

fix: check_any type error #1021

Closed
wants to merge 5 commits into from
Closed

fix: check_any type error #1021

wants to merge 5 commits into from

Conversation

KnownBlackHat
Copy link
Contributor

@KnownBlackHat KnownBlackHat commented May 4, 2023 •
edited
Loading

Summary


Fixes this type error

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, ...)

@KnownBlackHat KnownBlackHat marked this pull request as draft May 4, 2023 15:02
@shiftinv shiftinv added s: in progress Issue/PR is being worked on t: refactor/typing/lint Refactors, typing changes and/or linting changes t: bugfix labels May 4, 2023
@KnownBlackHat KnownBlackHat reopened this May 7, 2023
@KnownBlackHat KnownBlackHat force-pushed the master branch 2 times, most recently from 4ba514d to 613b347 Compare May 8, 2023 09:33
@KnownBlackHat KnownBlackHat marked this pull request as ready for review May 8, 2023 09:37
Skelmis
Skelmis reviewed May 8, 2023
View reviewed changes
Copy link
Contributor

@Skelmis Skelmis left a comment

Choose a reason for hiding this comment

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

Resolve actions also where applicable

disnake/ext/commands/_types.py
@@ -14,7 +14,9 @@
CoroFunc = Callable[..., Coro[Any]]

Check = Union[
Callable[["Cog", "Context[Any]"], MaybeCoro[bool]], Callable[["Context[Any]"], MaybeCoro[bool]]
Callable[["Cog"], Any],
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this not introduce ambiguity into types by making it so a check can take the cog as the first parameter and now can return anything it wishes to when items in checks should be callables that return a bool?

Copy link
Contributor Author

@KnownBlackHat KnownBlackHat May 8, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

yeah i am also trying to type hint in bool but the check inside check_any method is returning a Cog directly.

@KnownBlackHat KnownBlackHat marked this pull request as draft May 8, 2023 12:25
@shiftinv
Copy link
Member

shiftinv commented Jul 4, 2023

Thanks for the PR - I believe this is being superseded by #1048, which handles this more comprehensively

@shiftinv shiftinv closed this Jul 4, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Skelmis Skelmis Skelmis left review comments

Assignees
No one assigned
Labels
s: in progress Issue/PR is being worked on t: bugfix t: refactor/typing/lint Refactors, typing changes and/or linting changes
Projects
disnake
Status: In Progress
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@KnownBlackHat @shiftinv @Skelmis