Clarification needed on app_commands.Transformer
#9311
-
|
Very much a beginner here, so sorry if this comes off ignorant. Here's an excerpt from discord.py examples: class Point(NamedTuple):
x: int
y: int
class PointTransformer(app_commands.Transformer):
async def transform(self, interaction: discord.Interaction, value: str) -> Point:
(x, _, y) = value.partition(',')
return Point(x=int(x.strip()), y=int(y.strip()))
@client.tree.command()
async def graph(
interaction: discord.Interaction,
point: app_commands.Transform[Point, PointTransformer],
):
await interaction.response.send_message(str(point))What i have so far: class Urls():
urls:str
def __init__(self, urls):
self.urls:list = urls.split()
self.filter_imgurls()
#Sets self.img inside __init__. Also makes alterations to self.urls:list
def filter_imgurls(self) -> None:
...
#Returns a string derived from self.urls:list
def give_names(self, lntrn:str, sprtr:str) -> str:
...
class UrlsTransformer(app_commands.Transformer):
async def transform(self, interaction: discord.Interaction, value: str) -> Urls:
return await Urls(value)
def run():
...
@bot.tree.command()
async def test (
interaction:discord.Interaction,
urls:app_commands.Transform[Urls, UrlsTransformer]
):
await interaction.response.send_message(f"{urls.img}", ephemeral=True)
...
if __name__ == "__main__":
run()By itself, the class I want to "transform" command input to works just fine. But, so far, my attempts to hook it up to a Now, what I immediately notice is that What am i doing wrong? |
Beta Was this translation helpful? Give feedback.
Replies: 2 comments 5 replies
-
|
You should provide the actual message of your error You first annotate urls as string in the class body but then redefine the type as list in the body of |
Beta Was this translation helpful? Give feedback.
-
|
I'm sorry, but the problem is fixed now, and it would take too long for my taste to reproduce the error (my local git is a mess) To do the fix I just moved all the functions that my class did "inside itself" to the Transformer. You are probably right on point about What it does is as follows (I didn't change a thing, except the afformentioned shenanigans)... actually, let me just copypaste the entire currelt version of my import requests
from requests import Response
import typing
import discord
from discord import app_commands
...
class Urls(typing.NamedTuple):
data_urls: str
img_url: str
class UrlsTransformer(app_commands.Transformer):
async def transform(self, interaction:discord.Interaction, value:str) -> Urls:
def isurl(url:str) -> bool:
site_formats = ("https://", "http://")
if any(s in url for s in site_formats):
return True
return False
def filterimgurls(url:str) -> bool:
#filter image via HEAD requests
def fihr(url:str) -> bool:
image_formats = ("image/png", "image/jpeg", "image/jpg")
r:Response = requests.head(url, timeout=2)
if r.status_code not in range(200, 299):
return False
if r.headers["content-type"] in image_formats:
return True
return False
#Filter image via string manipulation
def fism(url:str) -> bool:
image_formats = ('.png', '.jpeg', '.jpg')
if any(s in url for s in image_formats):
return True
return False
if any([fihr(url), fism(url)]):
return True
return False
def give_names(url_list:list, lntrn:str, sprtr:str) -> str:
formed:list = []
def form(url:str, lntrn:str):
title = url.split("/")[2]
title = title.replace("www.", "")
out = f"{lntrn}[{title}]({url})"
return out
for el in url_list:
formed.append(form(el, lntrn))
out = f"{sprtr}".join(formed)
return out
def url_gen(url_list:list) -> typing.Generator:
for el in url_list:
yield el
def sort_urls(gen:typing.Generator, data_urls:list, img_urls:list) -> None:
for el in gen:
if isurl(el):
if filterimgurls(el):
img_urls.append(el)
else:
data_urls.append(el)
#!!!Do stuff from here!!!
urls = url_gen(value.split())
data_urls_list:list = []
img_urls_list:list = []
data_urls_out:str = ""
img_url_out:str = ""
#I hate it, but it works...
sort_urls(urls, data_urls_list, img_urls_list)
if data_urls_list:
data_urls_out = give_names(data_urls_list, "---", "\n")
if img_urls_list:
img_url_out = img_urls_list[0]
return Urls(data_urls=data_urls_out, img_url=img_url_out)Now, about setting I don't know if this IS, in fact, a bad practice, but it kinda felt wrong even as I was writing it, so I eventually got rid of it (the code above just returns I'd still want a less bulky But, hey, I have an excuse! I'm barely a week into coding. PS: Also, also, if you care to ellaborate on how my "type annotations are a bit weird"? |
Beta Was this translation helpful? Give feedback.
-
|
Yeah there is room for improvement i will only give a shallow code review i think others may chime in with other improvements
if any(s in url for s in site_formats):
return True
return FalseCan be shortened to: return (s in url for s in site_formats):It is redundant to check for the result of a boolean expression to then return the result of the same boolean expression
You already fixed that. Was just talking about how you first defined your There are probably also ways to improve the logic of your actual code but i currently don't really see how but i also didn't invest much time in trying to really understand your code right now |
Beta Was this translation helpful? Give feedback.
-
|
Wow. That's a lot. Thank you.
I will take a closer look into it, but I doubt if this will be worth it, since the project I'm working on is for a small community that can hardly overwhelm the bot with requests. And stuff from the example is too complex for me at the moment anyway.
I persomany find the latter version less intuitively understandable :/
Yeah. Sorry. Point taken. I will.
This is based purely on my "conseptual framework" not on actual need.
This is probably downstream from me not understanding some things about "scope" in general and also a consequence of me testing things outside before putting them into the Transformer. I have a bunch of ideas for improvement, one of which is exactly what you mentioned. I will probably get to it as one of the first ones.
I know. Sorry. I'm still figuring it out. For now the only two things I got straight is that if i'm writing class it should be capitalised with no
PS: Scratch that. Just read an article on class attributes. Don't know where I got the idea that those meant anything to the constructor. |
Beta Was this translation helpful? Give feedback.
-
It definitely is worth it, you should not introduce blocking code into your bot because it will add up with each feature you add that has blocking code. If only a few long blocking tasks happen together even on a small bot it will cause problems. Blocking means that discord.py won't respond to the heartbeat of the gateway which could cause your bot to be disconnected, all slash commands have to respond within 3 seconds to interactions or they automatically fail. Just two concurrent uses of this command could make your bot appear sluggish or make it disconnect in the worst case. I admit the example I linked is a big overkill you could simply just do an # in your main.py or bot.py whatever the file you are starting your bot with is named
bot = commands.Bot() # required args omitted for brevity and because they are not relevant to this example
# [...]
async def main():
async with aiohttp.ClientSession() as session, bot:
bot.session = session # attach an aiohttp session to the bot
discord.utils.setup_logging() # since we are using bot.start we have to setup default logging here
await bot.start(token)
# end of file
asyncio.run(main())and then use it like this in your transformer: async with interaction.client.session.head(url) as response:
# [...]
if response.headers['Content-Type'] in image_formats:
return True Yes it is more verbose but you definitely should not block your code for up to 2 seconds per command usage.
is_true = 1 + 1 == 2
if is_true:
return True
return Falseas opposed to return is_trueis considered a code smell everywhere, code analyzers will warn you to not do it. If you really consider that version better for yourself go ahead I just am telling you that it is universally considered bad coding style.
but for el in gen:later instead of just iterating through the list itself. This just seems useless and redundant to me since you are doing the direct for each loop just with extra steps.
Python has an official style guide (PEP-8) that most of the community is following to some degree (some stuff like line length is contested a lot but overall people will follow the majority on it) |
Beta Was this translation helpful? Give feedback.
-
|
I want to thank you again for your help. I still have some concerns about the particularities of the use of If you have time and/or desire to give me some feedback on how I implemented your advice, I'd very much apreciate it ^_^ So, this is what I have now...
#debug
if __name__ == "__main__":
from _findroot import *
#local
import settings
#discord
import discord
from discord import app_commands
#std libs
import logging
import typing
import asyncio
import aiohttp
from urllib.parse import unquote
import random
logger = logging.getLogger('transformer')
class _Pack(typing.NamedTuple):
reqest_str: str
response_obj: aiohttp.ClientResponse
class Urls:
def __init__(self, *, data_urls:typing.List[str]|None = None, img_urls:typing.List[str]|None = None) -> None:
self.__data_urls = data_urls
self.__img_urls = img_urls
def get_data_urls(self, bullet:str, separator:str) -> str|None:
title = lambda url: '.'.join(url.split('/')[2].split('.')[1:])
if self.__data_urls:
return separator.join([f"{bullet}[{title(url)}]({unquote(url)})" for url in self.__data_urls])
return None
def get_img_urls(self, bullet:str, click:str, separator:str) -> str|None:
if self.__img_urls:
return separator.join([f"{bullet}[{click}]({unquote(url)})" for url in self.__img_urls])
return None
def get_rng_image(self) -> str|None:
if not self.__img_urls:
return None
if len(self.__img_urls) == 1:
return self.__img_urls[0]
return random.choice(self.__img_urls)
class UrlsTransformer(app_commands.Transformer):
async def request_url(self, url:str, session: aiohttp.ClientSession) -> _Pack|None:
try:
async with session.get(url) as response: # -> response:aiohttp.ClientResponse
return _Pack(reqest_str=url, response_obj=response)
except aiohttp.client_exceptions.ClientConnectionError:
return None #don't mind multiple excepts...
except aiohttp.client_exceptions.InvalidURL:
return None #thees are just for logging
except aiohttp.client_exceptions.ClientError:
return None #which I've removed for this post
async def gather_request_urls(self, url_list: typing.List[str], session: aiohttp.ClientSession) -> typing.List[_Pack]:
u_l_no_repeats = [*set(url_list)]
output = await asyncio.gather(*[self.request_url(link, session) for link in u_l_no_repeats])
return [x for x in output if x != None]
async def sort_response(self, pack: _Pack, data_urls: typing.List[str], img_urls: typing.List[str]) -> None:
if pack.response_obj.content_type in settings.ALLOWED["content_types"]:
img_urls.append(pack.reqest_str)
elif any(s in pack.reqest_str for s in settings.ALLOWED["image_formats"]):
img_urls.append(pack.reqest_str)
else:
data_urls.append(pack.reqest_str)
async def gather_sort_responses(self, response_list: typing.List[_Pack], data_urls: typing.List[str], img_urls: typing.List[str]) -> None:
await asyncio.gather(*[self.sort_response(response_list[i], data_urls, img_urls) for i in range(len(response_list))])
async def transform(self, interaction:discord.Interaction, value:str) -> Urls:
links:list = [el.strip() for el in value.split()]
async with aiohttp.ClientSession() as session:
responses = await self.gather_request_urls(links, session)
images_out = []
data_out = []
if responses:
await self.gather_sort_responses(responses, data_out, images_out)
await session.close()
return Urls(
data_urls= data_out if data_out else None,
img_urls= images_out if images_out else None
) |
Beta Was this translation helpful? Give feedback.
-
I've heard of it, but haven't read it yet. Now it's moved closer to the top on my list. Thank you.
Ok. I will work on that. It seems to me that it just takes a moderate readjustment of perception, but if the pay off = making other people understand my code better and/or hide from them that I'm a gigantic amateur - I'm willing to try.
Well, it seemed at the time that removing items from the list without def remove_b(incoming:list, discarded:list) -> None:
for i, el in enumerate(incoming):
if el == 'b':
beees = incoming.pop(i)
discarded.append(beees)
my_list:list[str] = ['a', 'b', 'a', 'a', 'b', 'a', 'b', 'a', 'b']
junk:list = []
remove_b(my_list, junk)
print(my_list) #['a', 'a', 'a', 'a', 'a']
print(junk) #['b', 'b', 'b', 'b']I also wanted to do multiple "checks" and append discarded items to another list. Moreover, for some reason I didn't like my "main"(?) calling multiple "helpers"(?) consecutively... seemed redundant (imagine I had So, when I used the approach above, I noticed that pop causes my logger to "skip a beat" and if, for example, I had 4 items in a list it showed me only 3 operations from the loop (even though it got through all of them). This was probably unrelated to the aforementioned approach, but rather it was some other logical error on my part. I'm not sure what it was, but I couldn't find it at the time. Then I fooled around some more and discovered generators. Experimenting with them I figured that they did essentially the same thing while NOT making my logger confused and adding less "bulk" to the code (I guess). what I could have done is something like this (probably): def is_b(element:str):
return element == 'b'
def is_c(element:str):
return element == 'c'
my_list:list[str] = ['a', 'b', 'c', 'a', 'b', 'a', 'b', 'c', 'b']
my_other_list = [x for x in my_list if not any([is_b(x), is_c(x)])]
print(my_other_list ) # ['a', 'a', 'a']But It doesn't Also, I had a plan to return a And now you tell me that using |
Beta Was this translation helpful? Give feedback.
Yeah there is room for improvement i will only give a shallow code review i think others may chime in with other improvements
interaction.client.sessionsee https://github.com/Rapptz/discord.py/blob/master/examples/advanced_startup.py#L87 for an example on how to do something like that (ignore the asyncpg st…