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

ApplicationContext attributes have wrong type annotations #2635

Open
3 tasks done
probablyjassin opened this issue Nov 1, 2024 · 7 comments · May be fixed by #2636
Open
3 tasks done

ApplicationContext attributes have wrong type annotations #2635

probablyjassin opened this issue Nov 1, 2024 · 7 comments · May be fixed by #2636
Labels
unconfirmed bug A bug report that needs triaging

Comments

@probablyjassin
Copy link

probablyjassin commented Nov 1, 2024

Summary

The ctx attributes channel, user and message, guild, and possible more, are typed as methods, not attributes

Reproduction Steps

Minimal Reproducible Code

import os

import discord
from discord.ext import commands

from logger import setup_logger
from config import DISCORD_TOKEN
from models.CustomMogiContext import MogiApplicationContext

logger = setup_logger(__name__)

intents = discord.Intents.all()


class customBot(commands.Bot):
    def __init__(self, *args, **kwargs):
        return super().__init__(*args, **kwargs)

    @slash_command(name="test_issue")
    async def test_issue(self, ctx: ApplicationContext):
        await ctx.respond(ctx.channel.id)
        # this works, but `channel` has the type annotation of a method:
        # (method) def channel(self: Self@ApplicationContext) -> (InteractionChannel | None)

        channel = ctx.channel()
        await ctx.send(channel)
        # this however obviously doesn't work:
        # discord.errors.ApplicationCommandInvokeError: Application Command raised an exception: TypeError: 'TextChannel' object is not callable



bot = customBot(
    command_prefix=".",
    intents=intents,
)

bot.run(TOKEN)

Expected Results

Expected the type annotations to treat the ctx attributes channel, user and message as attributes.

And the syntax highlighting reflects them as such, causing following type annotations to break. (For example, theres no more autocomplete for ctx.channel.id, because the editor thinks channel is a method)

Actual Results

They get autocompleted as methods.

Intents

discord.Intents.all()

System Information

  • Python v3.12.7-final
  • py-cord v2.6.1-final
  • aiohttp v3.10.10
  • system info: Windows 11 10.0.22621

Checklist

  • I have searched the open issues for duplicates.
  • I have shown the entire traceback, if possible.
  • I have removed my token from display, if visible.

Additional Context

full traceback:

Traceback (most recent call last):
File "C:\Users\Me\MyProject\venv\Lib\site-packages\discord\bot.py", line 1149, in invoke_application_command
await ctx.command.invoke(ctx)
File "C:\Users\Me\MyProject\venv\Lib\site-packages\discord\commands\core.py", line 435, in invoke
await injected(ctx)
File "C:\Users\Me\MyProject\venv\Lib\site-packages\discord\commands\core.py", line 138, in wrapped
ret = await coro(arg)
^^^^^^^^^^^^^^^
File "C:\Users\Me\MyProject\venv\Lib\site-packages\discord\commands\core.py", line 1486, in _invoke
await command.invoke(ctx)
File "C:\Users\Me\MyProject\venv\Lib\site-packages\discord\commands\core.py", line 435, in invoke
await injected(ctx)
File "C:\Users\Me\MyProject\venv\Lib\site-packages\discord\commands\core.py", line 146, in wrapped
raise ApplicationCommandInvokeError(exc) from exc
discord.errors.ApplicationCommandInvokeError: Application Command raised an exception: TypeError: 'TextChannel' object is not callable

I went to the definition of ApplicationContext and saw that the properties with this problem have the decorator @cached_property
Replacing this with @Property fixes the issue, but of course I'm new to the codebase so this might not be the best way to fix this.

@property # instead of @cached_property
    def channel(self) -> InteractionChannel | None:
        """Union[:class:`abc.GuildChannel`, :class:`PartialMessageable`, :class:`Thread`]:
        Returns the channel associated with this context's command. Shorthand for :attr:`.Interaction.channel`.
        """
        return self.interaction.channel

so maybe this is the fix, or the definition of @cached_property needs some fixing

@probablyjassin probablyjassin added the unconfirmed bug A bug report that needs triaging label Nov 1, 2024
@probablyjassin
Copy link
Author

probablyjassin commented Nov 1, 2024

So i found the definition of @chached_property:

cached_property = NewType("cached_property", property)

Again as I'm not very familiar with the codebase, I don't know why this is a thing.

But simply replacing this with

cached_property = functools.cached_property

fixes this.

Is this a good solution or would this break something else?

@Paillat-dev
Copy link
Contributor

Indeed cached_property could either benefit of a better typing and or a better implementation. I can confirm that I experienced this issue myself, even tough it shouldn't happen.

Currently the implementation is as follows:

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

if TYPE_CHECKING:
    cached_property = NewType("cached_property", property)
else:
    cached_property = _cached_property

So the TYPE_CHECKING check should correctly type it as acting like a nomal property.
This could maybe be replaced with functools.cached_property might work, but idk if its implementation is incompatible.

@probablyjassin
Copy link
Author

@Paillat-dev thanks for your quick reply!

correct me if I misunderstand but what does NewType("cached_property", property) even mean in the first place? It's just a type derived from property, right?

So unless I'm misunderstanding some context, using functools.cached_property shouldn't be an issue.

I did try replacing it and that does fix the issue, and within my own discord bot it caused no other problems anywhere.

Could you find out if it's compatible, so that if it is, this can be implemented to fix the issue?
I've had this problem for a long time at this point.

@Paillat-dev
Copy link
Contributor

NewType("cached_property", property) at runtime returns property, but is interpreted as type checkers like the creation of a different type from property.

@Paillat-dev
Copy link
Contributor

image

@probablyjassin
Copy link
Author

I see. Yeah as part of this issue, usr has an Any type which sucks.

grafik

and using chached_property = functools.cached_property would indeed fix this:

grafik

@probablyjassin
Copy link
Author

#2636

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
unconfirmed bug A bug report that needs triaging
Projects
None yet
2 participants