fix: use property instead of NewType(...) to fix type annotations for ApplicationContext attributes

[probablyjassin]

fixes #2635 : use functools.cached_property to fix propert type annotations for context attributes

Summary

context attributes like guild, message, user and such have wrong type annotations. They are typed as methods instead of properties.
This is because a NewType based on property was used instead of functools.cached_property, like in other spots in the codebase already.
This PR implements functools.cached_property for context, which fixes these type annotations.

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 have updated the changelog to include these changes.

[probablyjassin]

Ah yeah that makes sense since it's not used anywhere else in the File, I forgot about that.

[Paillat-dev]

@probablyjassin Could you test your code by doing the following?

-class _cached_property:
-    def __init__(self, function):
-        self.function = function
-        self.__doc__ = getattr(function, "__doc__")
-
-    def __get__(self, instance, owner):
-        if instance is None:
-            return self
-
-        value = self.function(instance)
-       setattr(instance, self.function.__name__, value)
-
-        return value

+cached_property = functools.cached_property

if TYPE_CHECKING:
    from typing_extensions import ParamSpec

    from .abc import Snowflake
    from .commands.context import AutocompleteContext
    from .commands.options import OptionChoice
    from .invite import Invite
    from .permissions import Permissions
    from .template import Template

    class _RequestLike(Protocol):
        headers: Mapping[str, Any]

-    cached_property = NewType("cached_property", property)

    P = ParamSpec("P")

else:
-    cached_property = _cached_property
    AutocompleteContext = Any
    OptionChoice = Any

Try running a bot and doing a couple of things. If it works, push it to this pr, I will test it. It would allow to remove entirely pycord's cached_property implementation for the objectively more robust functools one.

[probablyjassin]

that would be very nice! i'll test it right now!

[probablyjassin]

@Paillat-dev here is what I found:

Your idea almost works perfectly, except that with these changes, the following line in context.py:

author: Member | User = user

causes this error:

TypeError: Cannot assign the same cached_property to two different names ('user' and 'author').

Full Stack Trace

Traceback (most recent call last): File "C:\Users\Jassin\AppData\Local\Programs\Python\Python311\Lib\functools.py", line 976, in __set_name__ raise TypeError( TypeError: Cannot assign the same cached_property to two different names ('user' and 'author').

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "C:\Users\Jassin\myprojectfolder\mybotfolder\main.py", line 3, in
import discord
File "C:\Users\Jassin\myprojectfolder\mybotfolder\venv\Lib\site-packages\discord_init_.py", line 34, in


Meaning, if a way can be found to assign ctx.author without just directly assigning user, this problem would be solved entirely.

So in addition to your idea, I tried this and it actually worked out I believe:

# context.py

- author: Member | User = user

+ @cached_property
+    def author(self) -> Member | User:
+        """Returns the user that sent this context's command.
+        Shorthand for :attr:`.Interaction.user`.
+        """
+        return self.interaction.user  # type: ignore # command user will never be None

I tested it on my end and it works well. So if you think this is a good solution, I'll add it to this PR.

[Paillat-dev]

I really start to wonder what the point is of cached_property in here. It has no utility whatsoever and could be replaced with property directly. I will see if I can find places where it serves an actual purpose, if not you might actually be able to remove it entirely everywhere in the codebase and replace it with simple propertyes

[DA-344]

It would allow to remove entirely pycord's cached_property implementation for the objectively more robust functools one.

functools.cached_property requires the classes to have a __dict__ mutable mapping, which almost all of the classes in this library lack for optimization. So making utils.cached_property just an alias for functools.cached_property will make cached properties to stop working. Current solution is OK.

Also, this decorator requires that the __dict__ attribute on each instance be a mutable mapping. This means it will not work with some types, such as metaclasses (since the __dict__ attributes on type instances are read-only proxies for the class namespace), and those that specify __slots__ without including __dict__ as one of the defined slots (as such classes don’t provide a __dict__ attribute at all).

This is why discord.utils.cached_property was a NewType(...) in the first place, in any case, it should be something like cached_property = property (or similar) to make type checkers happy.

[Paillat-dev]

You're right, I remember looking at it a couple of weeks ago, then it went out of my mind. In any case having the properties cached in almost all places makes almost no sense (see discussion here https://discord.com/channels/881207955029110855/881735314987708456/1304609838101172286), so it would be preferred to replace them with property directly.

I think for now @probablyjassin you can try as said by @DA-344 , cached_property = property and see if that works. I'll open an issue later about removing the unnecessary cached properties in favor of property.

[Paillat-dev]

Actually, @DA-344, I remembered why I didn't mention that slots thing, it is because pycord has utils.cached_property and utils.cached_slots_property. The first is used when __slots__ is not provided, where the latter for optimized classes.

But I think for now we can stop at just fixing the type checker issue and I'll move discussion to another issue.

[probablyjassin]

Alright, I replaced it with cached_property = property and from my quick testing it solves the issue without breaking functionality, nice!

[Paillat-dev]

I think this can be merged if the changed is approved as is.

[Paillat-dev]

@probablyjassin Can you change the pull request title to reflect the change ? Thx

[probablyjassin]

Done ✅

[DA-344]

it is because pycord has utils.cached_property and utils.cached_slots_property. The first is used when __slots__ is not provided, where the latter for optimized classes.

The cached_property decorator saves the result on the object itself when an instance is made, not on another attribute, which is what cached_slots_property does.
The functools.cached_property is essentially the same as cached_slot_property but using a not optimal mutable mapping, in which case it is __dict__.