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

Snowflake Protocol class Invalid Under mypy #8354

Closed
3 tasks done
ultrabear opened this issue Aug 19, 2022 · 5 comments
Closed
3 tasks done

Snowflake Protocol class Invalid Under mypy #8354

ultrabear opened this issue Aug 19, 2022 · 5 comments
Labels
suggestion A suggestion

Comments

@ultrabear
Copy link

Summary

The Snowflake class raises errors in mypy due to having a defined empty slots, when mypy reads the interface it expects types passed to it to also have empty slots, but as most dont mypy raises errors on them

Reproduction Steps

run the below code through the mypy typechecker (using discord.py==2.0)

Minimal Reproducible Code

import discord
from discord import User, Guild


async def ban_user_snowflake_error(guild: Guild, user: User) -> None:

    await guild.ban(user)

Expected Results

mypy should raise no error here, as does pyright

Actual Results

mypy raises the following error due to expecting the defined slots on User to be tuple[]

test.py:11: error: Argument 1 to "ban" of "Guild" has incompatible type "User"; expected "Snowflake"
test.py:11: note: Following member(s) of "User" have conflicts:
test.py:11: note:     __slots__: expected "Tuple[]", got "Tuple[str]"
Found 1 error in 1 file (checked 1 source file)

Intents

discord.Intents.all()

System Information

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

While I understand mypy is not supported by discord.py, the fix is as simple as removing __slots__ = () from the definition of Snowflake, and allows mypy to be used with discord.py again. Class init performance should not be a problem as Protocol subclasses may not be constructed at runtime.

@ultrabear ultrabear added the unconfirmed bug A bug report that needs triaging label Aug 19, 2022
@Gobot1234
Copy link
Contributor

Snowflake is used as a base class to concrete classes and as you said discord.py doesn't support mypy and it's recommended that you use pyright. I think if you want this fixed you should ask the mypy team.

@ultrabear
Copy link
Author

ultrabear commented Aug 19, 2022

According to this REPL run I just did the concrete classes do not actually subclass from Snowflake (at least User/Guild)

>>> import discord
>>> discord.User.mro()
[<class 'discord.user.User'>, <class 'discord.user.BaseUser'>, <class 'discord.user._UserTag'>, <class 'discord.abc.Messageable'>, <class 'object'>]
>>> discord.User.mro()[1].mro()
[<class 'discord.user.BaseUser'>, <class 'discord.user._UserTag'>, <class 'object'>]
>>> discord.Guild.mro()
[<class 'discord.guild.Guild'>, <class 'discord.mixins.Hashable'>, <class 'discord.mixins.EqualityComparable'>, <class 'object'>]
>>> 

Additionally in a patched version that comments out the __slots__ line User still has no __dict__ attribute

>>> print(inspect.getsource(discord.abc.Snowflake))
@runtime_checkable
class Snowflake(Protocol):
    """An ABC that details the common operations on a Discord model.

    Almost all :ref:`Discord models <discord_api_models>` meet this
    abstract base class.

    If you want to create a snowflake on your own, consider using
    :class:`.Object`.

    Attributes
    -----------
    id: :class:`int`
        The model's unique ID.
    """

    #__slots__ = ()
    id: int

>>> a = discord.User(data={'username': "foo", "id": "95", "discriminator": "4543", "avatar": "None"}, state={}).__dict__
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
AttributeError: 'User' object has no attribute '__dict__'. Did you mean: '__dir__'?

@ultrabear
Copy link
Author

An additional check using githubs class references feature on Snowflake shows that no classes outside of abc.py subclass Snowflake, and the only class that does subclass Snowflake (abc.User) also has no subclasses

@bijij
Copy link
Contributor

bijij commented Aug 19, 2022

Snowflake is a Protocol, and as such classes do not need to subclass it to be accepted as instances of it, they just need to match the signature. i.e. in the case of Snowflake have an attribute id of type int.

__slots__ exiting or not should not matter, it's a mypy issue and should be taken up with them.

@Rapptz Rapptz added suggestion A suggestion and removed unconfirmed bug A bug report that needs triaging labels Aug 19, 2022
@Rapptz
Copy link
Owner

Rapptz commented Aug 19, 2022

This is definitely a bug in mypy, but since the friction in fixing it is low enough I'll fix it.

@Rapptz Rapptz closed this as completed in 5fe54b3 Aug 19, 2022
dolfies pushed a commit to dolfies/discord.py-self that referenced this issue Mar 28, 2023
dolfies pushed a commit to dolfies/discord.py-self that referenced this issue Mar 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
suggestion A suggestion
Projects
None yet
Development

No branches or pull requests

4 participants