getavalon / core

The safe post-production pipeline - https://getavalon.github.io/2.0
MIT License
218 stars 49 forks source link

Expose tools to API #420

Open mottosso opened 5 years ago

mottosso commented 5 years ago

Goal

Enable customisation of tools display from external configurations.

Motivation

There are references to internal modules from within colorbleed-config that makes sense, but shouldn't be calling on members outside of Avalon's API (as it is now broken, since #418)

Implementation

Avalon's tools are fixed and can be relied upon, but not their implementation details. What can be relied upon is..

..but that's pretty much it. So how about..

Anything else a config requires?

mottosso commented 5 years ago

An alternative is making each tool package into an API in itself.

from avalon.tools import loader
loader.show()

At the cost of having that many more APIs to maintain.

davidlatwe commented 5 years ago

How about avalon.api.show("loader", parent=None) or avalon.api.show("loader", on=None) ? And here's the implementation pseudo code:

_registered_gui = {
    "loader": avalon.tools.loader,
    "creator": avalon.tools.creator,
    ...
}

def show(gui_name, on=None):
    _registered_gui[gui_name].show(parent=on)

# And maybe one could even register his/her own gui for any purpose
# as long as the method `show` is presented.
def register_gui(gui_name, gui_module):
    # Validate
    assert hasattr(gui_module, "show"), "GUI module should have method 'show' be exposed."
    assert callable(gui_module.show), "GUI method 'show' should be callable."
    # Register
    _registered_gui[gui_name] = gui_module

Or maybe use app_name, register_app, instead of gui.

mottosso commented 5 years ago

That's another option, yeah. We don't really have that level of indirection anywhere else though, so it might not be expected, but it's worth considering.

tokejepsen commented 5 years ago

I lean towards one API and David's suggestion of avalon.api.show.

But then we are also excluding any non-UI interaction with the tools and their libraries. For example just exposing the show method and #382 won't be possible. Not saying #382 has to be possible, but there might be interactions we can't think of right now.

davidlatwe commented 5 years ago

But then we are also excluding any non-UI interaction with the tools and their libraries.

To achieve that, I think adding them to avalon.util should be a good fit ? Like, a place for higher level tool set for convenient.

mottosso commented 5 years ago

But then we are also excluding any non-UI interaction with the tools and their libraries.

That is true, but I think we've already got a non-UI API, which is the current api.py and integration packages, so it should be fine. Programmatic interaction with a UI is a little backwards.

  1. Avalon interacts with disk and database
  2. API interacts with Avalon
  3. Developer interacts with API
  4. UI interacts with API
  5. Users interacts with UI

So for a function like opening the latest workfile, that's better suited at level 3. Cut out the middleman.

To achieve that, I think adding them to avalon.util should be a good fit ?

That might be a good idea; like the pyblish.util module; something inbetween a low-level API call, and high-level UI interaction.

An alternative is making each tool package into an API in itself.

I thought more about this and realised it would cause ambiguity.

from avalon import api
from avalon.tools import loader

class MyLoader(api.Loader):
  ...

Or is it loader.Loader? loader.Plugin? What is "loader" versus "Loader"?

How about avalon.api.show("loader", parent=None)

I gave this some thought as well, and thought of two issues.

  1. No code completion or linting of whether "loaderr" is spelled correctly or exists.
  2. Any additional interaction with a tool would need one more of these double-dispatch functions, e.g. show_and_close("publisher"), show_and_validate("publisher"), except now these only work with some tools, and not all. Which means we'll have to guess - especially without code completion - whether a function exists and whether it supports the tool.

avalon.api.show_creator(parent=None)

Granted, this isn't the most elegant or nice to look at solution, but I think it helps encourage a clear separation between what is meant for API access and what is meant for visual user interaction. So the rules would be:

Preferably, the show_ functions wouldn't return anything either, to guarantee people aren't building their pipelines on an unsupported API. But I can see how that's a little draconian, especially with regards to how often people have asked for the GUI handle back from Pyblish's show functions; so how about that for a compromise; the return value of show_* is the API for said tool. Then the class instance needs proper underscores for private members, and stable public members that - like any Avalon API - must never change. (So less is better).

from avalon import api
loader = api.show_loader()
loader.close()

Thoughts?

mottosso commented 5 years ago

Saw you mentioned this here some time ago @tokejepsen, good call.

mottosso commented 5 years ago

as it is now broken, since #418

I managed to fix this, and re-implement backwards compatibility, so 418 should no longer cause any issues (if it does, it's a bug).