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

Support providing option channel types as list #1000

Merged
merged 4 commits into from Apr 18, 2022

Conversation

Ratery
Copy link
Contributor

@Ratery Ratery commented Feb 12, 2022

Summary

Support providing option channel types as a list like [discord.TextChannel, discord.VoiceChannel].
Fixed #991.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • 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, typehinting, examples, ...)

@Dorukyum
Copy link
Member

I think we should force one type just like most of the other code.

@Ratery
Copy link
Contributor Author

Ratery commented Feb 13, 2022

I think we should force one type just like most of the other code.

Ok, but then we should update the example.

@Dorukyum
Copy link
Member

The example already uses a list, which I think is what we should use.
Don't do any changes though if other people disagree with this.

@Ratery
Copy link
Contributor Author

Ratery commented Feb 13, 2022

So, we should update examples or change the library code because it accepts only tuples but lists were used in that example.

@plun1331 plun1331 mentioned this pull request Feb 19, 2022
7 tasks
@Dorukyum
Copy link
Member

Dorukyum commented Feb 21, 2022

What I've previously meant is that maybe we could check for isinstance(passed, typing.Iterable) or some other way of checking for an iterable. Open for discussion.

@krittick krittick added bug Something isn't working Merge with squash priority: high High Priority status: in progress Work in Progess labels Mar 7, 2022
@krittick krittick added this to In progress in v2.0 via automation Mar 7, 2022
@krittick krittick added this to the v2.0 milestone Mar 7, 2022
@Dorukyum
Copy link
Member

Dorukyum commented Mar 8, 2022

Let's just use isinstance(datatype, list) for now, it would be more consistent with the rest of the library.

discord/enums.py Outdated Show resolved Hide resolved
discord/commands/options.py Outdated Show resolved Hide resolved
@Dorukyum Dorukyum requested a review from krittick April 5, 2022 14:36
@BobDotCom
Copy link
Member

We should use typing.Union[]

@krittick krittick requested a review from Dorukyum April 18, 2022 01:17
@Dorukyum
Copy link
Member

We should use typing.Union[]

Union support has been added to #1251.

@Lulalaby Lulalaby merged commit 87dff73 into Pycord-Development:master Apr 18, 2022
v2.0 automation moved this from In progress to Done Apr 18, 2022
krittick added a commit that referenced this pull request Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working priority: high High Priority status: in progress Work in Progess
Projects
No open projects
v2.0
  
Done
Development

Successfully merging this pull request may close these issues.

multiple-type arg option is beak
5 participants