quotientbot/quotient

[BUG] Invalid Argument + Unbound Variable

Closed this issue · 2 comments

https://github.com/quotientbot/Quotient-Bot/blob/main/src/utils/converters.py#L46-L52

async def convert(cls, ctx, arg):
    with contextlib.suppress(AttributeError):
        match = re.match(r"\(?(\d+),?\s*(\d+),?\s*(\d+)\)?", arg)
        check = all(0 <= int(x) <= 255 for x in match.groups())

    if match and check:
        return discord.Color.from_rgb([int(i) for i in match.groups()])

Color.from_rgb() takes three arguments.
Should be return discord.Color.from_rgb(*[int(i) for i in match.groups()])

If the contextlib really suppress the AttributeError then the variable match and check will be undefined.

deadaf commented

hey @rtk-rnjn thanks for pointing this out,
can you pls mention a test case where this will throw an error?

HI @deadaf

Test case:

  • Try setting embed color from 0,0,0 in command qembed, or any INT,INT,INT. By logic 0,0,0 should set embed color black. Instead it do nothing 👀 possibly logging error in the terminal with TypeError

Code Snippet

>>> import re
>>> from discord import Color
>>> 
>>> COLOR_RE = re.compile(r"\(?(\d+),?\s*(\d+),?\s*(\d+)\)?")  
>>> string = "120,20,21"
>>> match = COLOR_RE.match(string)
>>> 
>>> match.groups()
('120', '20', '21')
>>> 
>>> check = all(0 <= int(x) <= 255 for x in match.groups())
>>> check
True
>>>  
>>> bool(match and check)
True
>>> 
>>> Color.from_rgb([int(i) for i in match.groups()])
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
TypeError: Colour.from_rgb() missing 2 required positional arguments: 'g' and 'b'
>>> 

Unboud/NameError

I am not sure in what test case the Unbound/NameError could raise. But I strongly advice to avoid using suppress to define a variable. Infact the suppress is slower than try-except.

>>> from contextlib import suppress
>>> with suppress(ValueError):
...     x = int('a')
...
>>> print(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
NameError: name 'x' is not defined
>>>

try-except is much faster if compared to suppress both in error and non-error block

>>> from timeit import timeit
>>>
>>> # Error
>>> timeit("""with suppress(ValueError): int('a')""", setup="""from contextlib import suppress""")
0.8330175000010058
>>> timeit("""try: int('a')\nexcept ValueError: pass""")
0.5806096000014804
>>>
>>> # No Error
>>> timeit("""with suppress(ValueError): x = int(1)""", setup="""from contextlib import suppress""")
0.3652058000006946
>>> timeit("""try: int(1)\nexcept ValueError: pass""")
0.0913081000035163
>>>