Skip to content

Implement discord.Guild.get_or_fetch #1703

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

Closed
wants to merge 25 commits into from
Closed
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
3562b23
add get_or_fetch_member
TheGiga Oct 15, 2022
5b0c2f2
fix description
TheGiga Oct 15, 2022
5ad3f97
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 15, 2022
5da22ad
Merge branch 'Pycord-Development:master' into master
TheGiga Oct 17, 2022
97e3f7c
Merge branch 'master' into master
BobDotCom Oct 17, 2022
137e3a7
add discord.Guild.get_or_fetch()
Oct 17, 2022
4e22b71
Merge pull request #1 from TheGiga/guild.get_or_fetch
TheGiga Oct 17, 2022
b52b31f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 17, 2022
f6f8fab
fix docstring
TheGiga Oct 17, 2022
16110be
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 17, 2022
b8857a0
Merge branch 'master' into master
TheGiga Oct 18, 2022
a160243
switched from TypeVar to Union
TheGiga Oct 18, 2022
7245125
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 18, 2022
ee8732f
easiest typehinting possible i guess
TheGiga Oct 18, 2022
ec56c7b
back to typevar
TheGiga Oct 18, 2022
1faa45c
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 18, 2022
9ae4278
forgot to import thing uh
TheGiga Oct 18, 2022
d75257f
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 18, 2022
d9c5372
Merge branch 'master' into master
Lulalaby Oct 18, 2022
6af2205
Merge branch 'master' into master
BobDotCom Oct 26, 2022
7c2912e
Merge branch 'master' into master
BobDotCom Oct 26, 2022
be3f13c
Merge branch 'master' into master
Lulalaby Oct 30, 2022
be855bb
Merge branch 'master' into master
BobDotCom Nov 7, 2022
b455f61
Merge branch 'master' into master
BobDotCom Nov 7, 2022
0540992
Merge branch 'master' into master
BobDotCom Nov 7, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions discord/guild.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
Optional,
Sequence,
Tuple,
TypeVar,
Union,
overload,
)
Expand Down Expand Up @@ -895,6 +896,64 @@ def get_member(self, user_id: int, /) -> Member | None:
"""
return self._members.get(user_id)

_FETCHABLE = TypeVar(
Copy link
Contributor

Choose a reason for hiding this comment

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

Usually type variables are defined at the very top of a Python file, is there any specific reason it's defined in the Guild class here?

"_FETCHABLE",
bound=Union[
VoiceChannel,
TextChannel,
ForumChannel,
StageChannel,
CategoryChannel,
Thread,
Member,
],
)

async def get_or_fetch(
self, object_type: Type[_FETCHABLE], object_id: int, /
) -> _FETCHABLE | None:
"""Shortcut method to get data from guild object if it's cached or fetch from api if it's not.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"""Shortcut method to get data from guild object if it's cached or fetch from api if it's not.
"""Shortcut method to get data from guild object either by returning the cached version, or if it does not exist, attempt to fetch it from the api.


Parameters
----------
object_type: :class:`Type[VoiceChannel, TextChannel, ForumChannel, StageChannel, CategoryChannel, Thread, Member]`
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
object_type: :class:`Type[VoiceChannel, TextChannel, ForumChannel, StageChannel, CategoryChannel, Thread, Member]`
object_type: Union[:class:`VoiceChannel`, :class:`TextChannel`, :class:`ForumChannel`, :class:`StageChannel`, :class:`CategoryChannel`, :class:`Thread`, :class:`Member`]

Type of object to fetch or get.

object_id: :class:`int`
ID of object to get.

Returns
-------

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change

Optional[:class:`~Type[VoiceChannel, TextChannel, ForumChannel, StageChannel, CategoryChannel, Thread, Member]`]
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
Optional[:class:`~Type[VoiceChannel, TextChannel, ForumChannel, StageChannel, CategoryChannel, Thread, Member]`]
Optional[Union[:class:`VoiceChannel`, :class:`TextChannel`, :class:`ForumChannel`, :class:`StageChannel`, :class:`CategoryChannel`, :class:`Thread`, :class:`Member`]]

The object of type that was specified or ``None`` if not found.

Usage
-----
``await GUILD_OBJECT.get_or_fetch(OBJECT_TYPE, OBJECT_ID)``
Comment on lines +931 to +933
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the point of this? It should be very obvious by reading the rest of the documentation for this method. Instead, include a proper example (you can look at other portions of the code to see how examples are showcased).

"""

def get_attr_name(t: object_type) -> str:
Copy link
Contributor

Choose a reason for hiding this comment

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

What was the point of even including the main portion of the code in a nested function? get_or_fetch doesn't seem to take in a function argument that it can use to derive the name of the attribute.

if t is Member:
return "member"
elif t in [
VoiceChannel,
TextChannel,
ForumChannel,
StageChannel,
CategoryChannel,
Thread,
]:
Comment on lines +939 to +946
Copy link
Contributor

Choose a reason for hiding this comment

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

The list of channel classes here should be a tuple because you do not need mutability here.

return "channel"

raise InvalidArgument(
f"Class {object_type} cannot be used with discord.Guild.get_or_fetch()"
Copy link
Contributor

Choose a reason for hiding this comment

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

You should use the name of the class (__name__) here so the exception looks more human friendly.

)

return await utils.get_or_fetch(
obj=self, attr=get_attr_name(object_type), id=object_id, default=None
)

@property
def premium_subscribers(self) -> list[Member]:
"""List[:class:`Member`]: A list of members who have "boosted" this guild."""
Expand Down