pyblish / pyblish-base

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

Implement #250 #325

Open BigRoy opened 7 years ago

BigRoy commented 7 years ago

This implements #250 where ContextPlugins that filter to specific families (so "*" not in plugin.families) will not run when no instance present that the given family.

When the ContextPlugin filters to two families and both are present by instances the ContextPlugin will still only run once.

import pyblish.api

class Collector(pyblish.api.ContextPlugin):
    order = pyblish.api.CollectorOrder

    def process(self, context):
        instance = context.create_instance("a1")
        instance.data['families'] = ["a"]

        instance = context.create_instance("a2")
        instance.data['families'] = ["a"]

        instance = context.create_instance("c1")
        instance.data['families'] = ["c"]

class ValidateA(pyblish.api.ContextPlugin):
    families = ["a"]
    order = pyblish.api.CollectorOrder

    def process(self, context):
        print("Processing %s" % self.__class__.__name__)

class ValidateB(pyblish.api.ContextPlugin):
    families = ["b"]
    order = pyblish.api.CollectorOrder

    def process(self, context):
        print("Processing %s" % self.__class__.__name__)

class ValidateAC(pyblish.api.ContextPlugin):
    families = ["a", "c"]
    order = pyblish.api.CollectorOrder

    def process(self, context):
        print("Processing %s" % self.__class__.__name__)

class ValidateAlways(pyblish.api.ContextPlugin):
    order = pyblish.api.CollectorOrder

    def process(self, context):
        print("Processing %s" % self.__class__.__name__)

pyblish.plugin.deregister_all_plugins()
pyblish.plugin.deregister_all_paths()
pyblish.plugin.deregister_all_hosts()
pyblish.plugin.deregister_all_callbacks()
pyblish.plugin.deregister_all_targets()
pyblish.api.register_plugin(Collector)
pyblish.api.register_plugin(ValidateA)
pyblish.api.register_plugin(ValidateB)
pyblish.api.register_plugin(ValidateAC)
pyblish.api.register_plugin(ValidateAlways)
pyblish.util.publish()

# Processing ValidateAC
# Processing ValidateA
# Processing ValidateAlways
BigRoy commented 7 years ago

The tests seem to fail because of:

======================================================================
FAIL: Context is processed regardless of families

----------------------------------------------------------------------

Traceback (most recent call last):
File "/home/travis/virtualenv/python2.6.9/lib/python2.6/site-packages/nose/case.py", line 197, in runTes
self.test(*self.arg)
File "/home/travis/build/pyblish/pyblish-base/tests/pre13/test_logic.py", line 174, in test_incompatible_context
assert_equals(count["#"], 1)
AssertionError: 0 != 1

Funnily enough that test sounds like the exact opposite of what we want with #250 so it should be correct the test is failing on this.

Particulary this:

    # Even with a family, if an instance is not requested
    # it still processes.
    class ValidateContext(pyblish.api.Validator):
        families = ["NOT_EXIST"]

        def process(self, context):
            count["#"] += 1
BigRoy commented 6 years ago

Was this ever solved with another PR - or @tokejepsen I remember you mentioned you were having problems with the ContextPlugins always being picked up before too - which this PR would've solved.

Any reason this is still open without any comments? :) The reason could be this comment?

tokejepsen commented 6 years ago

Think I've solved my issue with ContextPlugins by doing this family check early in the plugin.

Would be nice with this feature :)

BigRoy commented 6 years ago

@mottosso any pointers on how you feel it's best to proceed with this? we're still hitting this issue here and there.

tokejepsen commented 6 years ago

Think the biggest issue will be backward compatibility on this.

Maybe it needs to be tackle with an opting-in environment variable, similar to https://github.com/pyblish/pyblish-base/pull/332.

mottosso commented 6 years ago

Yes, an opt-in would be preferable.

The way forwards could be to keep adding opt-ins, till we eventually find equilibrium with a few opt-ins being permanently opted in at which point it's time for a 2.0 (where it's OK to break backwards compatibility).

For this feature, the important thing is that it works the same in Lite and QML as well (which may already happen as a result of implement it for base).

BigRoy commented 6 years ago

Thanks for confirming. I hope to find some time to take a look this week and get this fixed for all. :+1:

mottosso commented 6 years ago

Let's collect opt-in environment variables like this. Then I've got some ideas for a better way of documenting the API.

BigRoy commented 6 years ago

Woop, there it is.


@mottosso @tokejepsen You guys probably know about the following:

Remember me again why pyblish_qml and pyblish_lite have their own iterator logic?

Why couldn't it just use pyblish.logic.Iterator? The only difference I'm seeing is that the iterator in pyblish-base always ignores plug-ins that are not set to active.

But wouldn't (shouldn't?) the GUI do the same? Because it would just "hold" the rest of the iterator after collection until one decides to continue to process. In the meantime the user could change possible states (like active of the plug-ins)... thus the iterator would behave as expected.

mottosso commented 6 years ago

It's because the GUI needs to know which plugin/instance pair runs, before the iterator has returned it. The iterator only returns the result of an operation.

It should be more unified, I just never managed to get it working.

RafaelVillar commented 6 years ago

Checking on the pulse of this issue. Would absolutely be great to have contextPlugins respect families.

tokejepsen commented 6 years ago

This PR should probably be updated to respect https://api.pyblish.com/pages/PYBLISH_EARLY_ADOPTER.html

mottosso commented 6 years ago

Yes! It's very possible to implement this now; either as an early-adopter feature (i.e. asserting that it will be the default of 2.0, still to be discussed) or as an opt-in feature (possibly also available but not default in 2.0).

mottosso commented 6 years ago

I think the deprecation system in Maya 2018 can be a good fit for us.

In a nutshell, it has a flag to disable all deprecated functionality, throwing errors when they're used. Basically the opposite of an opt-in. In our case, it'd be "services" and old "CollectorPlugin" etc. And, if this were to be a winner, the old behavior of contexts not respecting families.

RafaelVillar commented 6 years ago

Thank you for the prompt responses. Glad to see there is support for this in the works. It looks like it is waiting in a pr?

mottosso commented 6 years ago

This is a PR. :) You can give Roy's fork a try, see if it achieves what you're looking for.