Implement `discord.Guild.get_or_fetch` #1703

TheGiga · 2022-10-15T19:12:18Z

Summary

Adds shortcut get_or_fetch method to discord.Guild. Shortcut to fetch stuff easily.

Information

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

Checklist

I have searched the open pull requests for duplicates.
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.

i hope it works lol

for more information, see https://pre-commit.ci

codecov · 2022-10-15T19:16:29Z

Codecov Report

Merging #1703 (0540992) into master (6b84ae9) will decrease coverage by 0.00%.
The diff coverage is 22.22%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #1703      +/-   ##
==========================================
- Coverage   33.14%   33.13%   -0.01%     
==========================================
  Files          96       96              
  Lines       18513    18522       +9     
==========================================
+ Hits         6136     6138       +2     
- Misses      12377    12384       +7

Flag	Coverage Δ
macos-latest-3.10	`33.12% <22.22%> (-0.01%)`	⬇️
macos-latest-3.11	`33.12% <22.22%> (-0.01%)`	⬇️
macos-latest-3.8	`33.13% <22.22%> (-0.01%)`	⬇️
macos-latest-3.9	`33.13% <22.22%> (-0.01%)`	⬇️
ubuntu-latest-3.10	`33.12% <22.22%> (-0.01%)`	⬇️
ubuntu-latest-3.11	`33.12% <22.22%> (-0.01%)`	⬇️
ubuntu-latest-3.8	`33.13% <22.22%> (-0.01%)`	⬇️
ubuntu-latest-3.9	`33.13% <22.22%> (-0.01%)`	⬇️
windows-latest-3.10	`33.12% <22.22%> (-0.01%)`	⬇️
windows-latest-3.11	`33.12% <22.22%> (-0.01%)`	⬇️
windows-latest-3.8	`33.13% <22.22%> (-0.01%)`	⬇️
windows-latest-3.9	`33.13% <22.22%> (-0.01%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
discord/guild.py	`23.81% <22.22%> (-0.02%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6b84ae9...0540992. Read the comment docs.

BobDotCom · 2022-10-15T21:01:13Z

Since these are just shortcut methods, I think we should probably figure out a better long-term solution. Maybe a helper method, Guild.get_or_fetch("user", 123) or even Guild.get_or_fetch(User, 123). This could also handle other objects besides users and could even be added to other objects like Client. A further discussion in the discord might be helpful (See thread).

TheGiga · 2022-10-17T20:15:39Z

Since it's planned to make universal get_or_fetch method, is this PR still needed?

NeloBlivion · 2022-10-17T20:24:45Z

Since it's planned to make universal get_or_fetch method, is this PR still needed?

The universal get_or_fetch method has always existed under utils, but it was only documented recently in #1682
Even though it exists, having these aliases is ultimately for convenience and has already been seen before with Client.get_or_fetch_user

pycord/discord/client.py

Line 1017 in 75bc6fe

return await utils.get_or_fetch(obj=self, attr="user", id=id, default=None)

add discord.Guild.get_or_fetch()

for more information, see https://pre-commit.ci

why can't people just see spelling mistakes before commiting

for more information, see https://pre-commit.ci

TheGiga · 2022-10-17T22:32:35Z

I'm not exactly sure about implementation, it seems pretty shitty, but it's the only way i made this thing work.
I'm may be missing something, but TypeVar doesn't throw an error if you pass into argument something that is not declared in that same TypeVar.

discord/guild.py

for more information, see https://pre-commit.ci

TheGiga · 2022-10-18T18:43:30Z

Union just have better type hinting for this specific case, using bound didn't help with type checking. May be i am still missing on something.

BobDotCom · 2022-10-18T19:17:01Z

Union just have better type hinting for this specific case, using bound didn't help with type checking. May be i am still missing on something.

Using bound does work. You need to use bound=Union[...].

for more information, see https://pre-commit.ci

TheGiga · 2022-10-18T19:33:38Z

Leaving it as is (it works). Waiting for Bob's PR about docstrings and stuff.

for more information, see https://pre-commit.ci

BobDotCom · 2022-11-07T17:41:30Z

I've been thinking, would it be better to add a use_cached kwarg to all fetch_xx methods? It would use the same functionality as get_or_fetch and would be much cleaner IMO.

EmreTech · 2022-11-08T05:38:23Z

discord/guild.py

+            elif t in [
+                VoiceChannel,
+                TextChannel,
+                ForumChannel,
+                StageChannel,
+                CategoryChannel,
+                Thread,
+            ]:


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

EmreTech · 2022-11-08T05:39:43Z

discord/guild.py

+                return "channel"
+
+            raise InvalidArgument(
+                f"Class {object_type} cannot be used with discord.Guild.get_or_fetch()"


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

EmreTech · 2022-11-08T05:40:40Z

discord/guild.py

@@ -895,6 +896,64 @@ def get_member(self, user_id: int, /) -> Member | None:
        """
        return self._members.get(user_id)

+    _FETCHABLE = TypeVar(


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?

EmreTech · 2022-11-08T05:42:09Z

discord/guild.py

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


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`]

EmreTech · 2022-11-08T05:43:06Z

discord/guild.py

+    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.


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.

EmreTech · 2022-11-08T05:43:32Z

discord/guild.py

+        Returns
+        -------
+
+        Optional[:class:`~Type[VoiceChannel, TextChannel, ForumChannel, StageChannel, CategoryChannel, Thread, Member]`]


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`]]

EmreTech · 2022-11-08T05:43:41Z

discord/guild.py

+
+        Returns
+        -------
+


Suggested change

EmreTech · 2022-11-08T05:44:50Z

discord/guild.py

+        Usage
+        -----
+        ``await GUILD_OBJECT.get_or_fetch(OBJECT_TYPE, OBJECT_ID)``


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

EmreTech · 2022-11-08T05:45:58Z

discord/guild.py

+        ``await GUILD_OBJECT.get_or_fetch(OBJECT_TYPE, OBJECT_ID)``
+        """
+
+        def get_attr_name(t: object_type) -> str:


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.

EmreTech · 2022-12-01T04:04:56Z

This PR description is outdated. The description talks about implementing a get_or_fetch_member method, but this PR now implements a get_or_fetch method.

Lulalaby · 2023-01-05T19:02:33Z

stale

TheGiga and others added 3 commits October 15, 2022 19:05

add get_or_fetch_member

3562b23

i hope it works lol

fix description

5b0c2f2

[pre-commit.ci] auto fixes from pre-commit.com hooks

5ad3f97

for more information, see https://pre-commit.ci

Lulalaby added priority: medium Medium Priority feature Implements a feature labels Oct 16, 2022

Merge branch 'Pycord-Development:master' into master

5da22ad

BobDotCom added the more info required more information is required to proceed label Oct 17, 2022

Merge branch 'master' into master

97e3f7c

BobDotCom marked this pull request as draft October 17, 2022 19:20

TheGiga and others added 5 commits October 18, 2022 01:20

add discord.Guild.get_or_fetch()

137e3a7

Merge pull request #1 from TheGiga/guild.get_or_fetch

4e22b71

add discord.Guild.get_or_fetch()

[pre-commit.ci] auto fixes from pre-commit.com hooks

b52b31f

for more information, see https://pre-commit.ci

fix docstring

f6f8fab

why can't people just see spelling mistakes before commiting

[pre-commit.ci] auto fixes from pre-commit.com hooks

16110be

for more information, see https://pre-commit.ci

TheGiga changed the title ~~Add get_or_fetch_member to discord.Guild~~ Implement discord.Guild.get_or_fetch Oct 17, 2022

TheGiga marked this pull request as ready for review October 17, 2022 22:35

Merge branch 'master' into master

b8857a0

BobDotCom requested changes Oct 18, 2022

View reviewed changes

discord/guild.py Outdated Show resolved Hide resolved

BobDotCom added status: in progress Work in Progess and removed more info required more information is required to proceed labels Oct 18, 2022

Dorukyum requested changes Oct 18, 2022

View reviewed changes

discord/guild.py Show resolved Hide resolved

discord/guild.py Outdated Show resolved Hide resolved

discord/guild.py Outdated Show resolved Hide resolved

TheGiga and others added 2 commits October 18, 2022 18:41

switched from TypeVar to Union

a160243

[pre-commit.ci] auto fixes from pre-commit.com hooks

7245125

for more information, see https://pre-commit.ci

easiest typehinting possible i guess

ee8732f

TheGiga requested review from Dorukyum and BobDotCom and removed request for Dorukyum and BobDotCom October 18, 2022 18:59

TheGiga and others added 2 commits October 18, 2022 19:32

back to typevar

ec56c7b

[pre-commit.ci] auto fixes from pre-commit.com hooks

1faa45c

for more information, see https://pre-commit.ci

TheGiga and others added 3 commits October 18, 2022 19:36

forgot to import thing uh

9ae4278

[pre-commit.ci] auto fixes from pre-commit.com hooks

d75257f

for more information, see https://pre-commit.ci

Merge branch 'master' into master

d9c5372

BobDotCom marked this pull request as draft October 19, 2022 17:34

BobDotCom and others added 4 commits October 26, 2022 11:44

Merge branch 'master' into master

6af2205

Merge branch 'master' into master

7c2912e

Merge branch 'master' into master

be3f13c

Merge branch 'master' into master

be855bb

BobDotCom added 2 commits November 7, 2022 17:29

Merge branch 'master' into master

b455f61

Merge branch 'master' into master

0540992

EmreTech reviewed Nov 8, 2022

View reviewed changes

Lulalaby closed this Jan 5, 2023

BobDotCom mentioned this pull request Jan 5, 2023

Implement better get_or_fetch #1856

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Implement `discord.Guild.get_or_fetch` #1703

Implement `discord.Guild.get_or_fetch` #1703

TheGiga commented Oct 15, 2022 •

edited

codecov bot commented Oct 15, 2022 •

edited

BobDotCom commented Oct 15, 2022 •

edited by Middledot

TheGiga commented Oct 17, 2022

NeloBlivion commented Oct 17, 2022

TheGiga commented Oct 17, 2022

TheGiga commented Oct 18, 2022

BobDotCom commented Oct 18, 2022

TheGiga commented Oct 18, 2022

BobDotCom commented Nov 7, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech Nov 8, 2022

EmreTech commented Dec 1, 2022

Lulalaby commented Jan 5, 2023

	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`]

	"""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.

	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`]]

Implement discord.Guild.get_or_fetch #1703

Implement discord.Guild.get_or_fetch #1703

Conversation

TheGiga commented Oct 15, 2022 • edited

Summary

Information

Checklist

codecov bot commented Oct 15, 2022 • edited

Codecov Report

BobDotCom commented Oct 15, 2022 • edited by Middledot

TheGiga commented Oct 17, 2022

NeloBlivion commented Oct 17, 2022

TheGiga commented Oct 17, 2022

TheGiga commented Oct 18, 2022

BobDotCom commented Oct 18, 2022

TheGiga commented Oct 18, 2022

BobDotCom commented Nov 7, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

EmreTech commented Dec 1, 2022

Lulalaby commented Jan 5, 2023

Implement `discord.Guild.get_or_fetch` #1703

Implement `discord.Guild.get_or_fetch` #1703

TheGiga commented Oct 15, 2022 •

edited

codecov bot commented Oct 15, 2022 •

edited

BobDotCom commented Oct 15, 2022 •

edited by Middledot