-
-
Notifications
You must be signed in to change notification settings - Fork 479
feat: ✨ Add fetch_roles_member_counts method to Guild and corresponding HTTP route
#3020
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
base: master
Are you sure you want to change the base?
Conversation
|
Thanks for opening this pull request! This pull request can be checked-out with: git fetch origin pull/3020/head:pr-3020
git checkout pr-3020This pull request can be installed with: pip install git+https://github.com/Pycord-Development/pycord@refs/pull/3020/head |
fetch_roles_member_counts method to Guild and corresponding HTTP routefetch_roles_member_counts method to Guild and corresponding HTTP route
| async def fetch_roles_member_counts(self) -> dict[int, int]: | ||
| """|coro| | ||
| Fetches a mapping of role IDs to their member counts for this guild. | ||
| Returns | ||
| ------- | ||
| Dict[:class:`int`, :class:`int`] | ||
| A mapping of role IDs to their member counts. | ||
| """ | ||
| r = await self._state.http.get_roles_member_counts(self.id) | ||
| return {int(role_id): count for role_id, count in r.items()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be a better ux if we map it to Role: count instead of only the IDs? The role should be cached anyways, but we can fall back to Object if needed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I did think about that. I'm going to see again for mapping a role object, it just seemed weird to me but we could do that. As for Role.count I don't think that would be a good thing, it's not really what it is, you get it separately, it's not on the role object.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, no Role.count wouldn't make sense and potentially even footgun us when Discord makes that an actual thing but with a different purpose.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The only thing with mapping ROle / Object instead of id is that it would force the user to create a discord.Object if they only have an id
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you mean? The library will be returning either Role or Object as the key, not the user?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By user I meant user of the library.
Basically, if they have some role id and they want to get its count, they'd need to create a discord.Object first (should be enough because it and Role both implement Hashable)
Summary
depends on discord/discord-api-docs#8021
Ref: https://discord.com/channels/881207955029110855/1411737823635312693/1446851187625164827
Information
examples, ...).
Checklist
type: ignorecomments were used, a comment is also left explaining why.