pyblish / pyblish-base

Pyblish base library - see https://github.com/pyblish/pyblish for details.
Other
127 stars 59 forks source link

Rename Callback to Handler #293

Open mottosso opened 8 years ago

mottosso commented 8 years ago

Goal

This is a purely cosmetic change, but I think also an important difference in mindset; emit signals and handle them.

Motivation

A "callback" suggests running a function and having another function get "called back" once it's done. It's common in Javascript, which is asynchronous, for things like making a request and passing along a nameless function (i.e. lambda) that is to be run once it's done.

This isn't what is happening in Pyblish.

Instead, callbacks are more closely related to signals and slots of Qt, and I think we should treat them that way.

Implementation

register_callback should instead beregister_handler. Alternatively, perhaps more succintly as pyblish.api.on("someEvent", my_handler)

from pyblish import api

def on_toggled(instance, before, after):
  pass

# Register
api.on("instanceToggled", on_toggled)

# Deregister
del(on_toggled)

# Get
api.on("instanceToggled")  # == (on_toggled,)

Backwards Compatibility

We can maintain both for the time being, mark the current one as deprecated, and let it be. It doesn't change what they do or how they are emitted and should be relatively painless to transition to.

tokejepsen commented 8 years ago

A "callback" suggests running a function and having another function get "called back" once it's done.

Makes sense, I'm in:)

pyblish.api.on("someEvent", my_handler)

Is this implemented elsewhere, or is this to follow more of a Qt signal/slots behaviour?

We can maintain both for the time being, mark the current one as deprecated, and let it be. It doesn't change what they do or how they are emitted and should be relatively painless to transition to.

This is becoming a higher and higher priority; https://github.com/pyblish/pyblish-qml/issues/165 to be able to keep up with deprecation :) Maybe we should be able to search our code base for deprecation? (sorry about derailing the topic)

mottosso commented 8 years ago

Is this implemented elsewhere, or is this to follow more of a Qt signal/slots behaviour?

If you think about it, it's literally renaming the function register_callback to on. In that way, yes, I'd say it's already implemented. I just think it makes it easier to think about conceptually.

Maybe we should be able to search our code base for deprecation?

Deprecated functionality is being tagged with a decorator currently, that spits out a warning when anything deprecated is being used. Now in theory, you could simply enable those warnings, and it'd tell you what and where you are using anything like that, but it hasn't seen much use and I'm pretty sure the base library is already using that internally (via util.py) for backwards compatibility, so it'll probably spit out a lot of stuff.

Here's an example.

$ python -Wdefault -c "import pyblish.api;pyblish.api.discover()"
c:\pythonpath\pyblish\plugin.py:946: DeprecationWarning: Call to deprecated function register_service.
  def register_service(name, obj):
c:\pythonpath\pyblish\plugin.py:946: DeprecationWarning: Call to deprecated function register_service.
  def register_service(name, obj):
c:\pythonpath\pyblish\plugin.py:946: DeprecationWarning: Call to deprecated function register_service.
  def register_service(name, obj):
c:\pythonpath\pyblish\plugin.py:946: DeprecationWarning: Call to deprecated function register_service.
  def register_service(name, obj):

Feel free to pop up an issue about it, first in pyblish-base to iron out the kinks, and then we can have a look at how to propagate it to a visual representation in pyblish-qml.