shizmob/pydle

Callback argument names are hard to understand for new users

Closed this issue ยท 8 comments

When I receive a message from a channel, my callback are called. In README the function is declared as:

def on_message(self, source, target, message)

In the doc I see:

on_message(target, by, message)

Then, I can guess that by is the user who sent this message, but what is source, and what is target? Why they mismatch here? A target is the message sender but a source is a target?

It takes me the whole afternoon to understand this what's-what to fix my (should-be-)simple bot.

It would be clear to me if they were named sender / user and channel, or explained in the docs (and be consistent).

The on_message callback is defined in rfc1549/client.py and uses the following signature:

   @async.coroutine
   def on_message(self, target, by, message):
        """ Callback called when the client received a message. """
        pass

target refers to the target of the message, if the message was sent in a channel it is the name of the channel. If the target was a user, the name of the user fills this field.

by is the name of the user that sent the message.
message is the content of the message.

It does appear there is a discrepancy in the readme where the arguments are out of order.

This can be corrected with a small fix to the readme.

If you are unsure about the order in which arguments are passed to a function, you can always do something like this:

def foo(*args, **kwargs):
    print(f"args={args}\tkwargs={kwargs}")

and work backwards

Actually, i just noticed the issue is already corrected on the asyncio branch, see #50 .
The issue seems to be you worked off the master branch rather than asyncio, which doesn't have the fix from #50

Thanks, I didn't know there is an asyncio branch. I was in a hurry to stop spam in my channel.

It would be nicer if you put the explaination in the docs. And I'm now searching and reading the source code rather than the docs for how to do things.

Yes, the docs are a bit confusing on that point, apologies. I will in fact clean them up, as well as merge asyncio to master.

I'm looking forward to it ๐Ÿ‘ Every time I restart my program I get a traceback from Tornado that I have no idea to clean up.

unless you have built your application expressly in Tornado, you can swap out the default branch for the asyncio one.

# remove existing pydle install from env
pip uninstall pydle 
# install asyncio branch pydle
pip install -e git://github.com/Shizmob/pydle.git@asyncio#egg=pydle

Temporary solution til asyncio gets merged into master and PyPy gets updated ๐Ÿ˜‰

This specific issue is fixed now, since 450ed75 has been merged into the master branch.

Yep. Should the documentation still be confusing please do open a new ticket!

Closing as resolved