API changes


#9

2. Services

DI opens up doors for added functionality not before possible.

def process(self, context, instance):
  # the function is given the current Context, and Instance

The above mimics the current behaviour, with slightly less typing and options for excluding both Context and Instance from the function signature where needed.

But it also means the ability to inject custom functionality.

def process(self, instance, time, user):
  print("%s was published @ %s by %s" % instance.data("name"), time(), user)

In which time and user are injected on-demand, providing additional functionality to the plug-in. In this case, a callable function time which returns the current time, and a static value user.

Furthermore, services can be registered by developers.

import pyblish.api
pyblish.api.register_service(
  "time", lambda: datetime.datetime.now().strftime("%Y-%m-%dT%H:%M:%S.%fZ"))

In the above, a custom service time is registered and made available to plug-ins, providing a pre-formatted version the current time, such that every plug-in uses the same formatting and needn’t concern themselves with maintaining any updates to it.

Services vs. Data

Where does the line go between what is data and what is a service?

If data, added via e.g. Context.set_data(key, value), represents data shared amongst plug-ins, services may represent shared functionality.

Though there is technically nothing preventing you from storing callables as data…

import time
context.set_data("time", lambda: time.time)

Just as there is technically nothing preventing you from providing constants as a service.

pyblish.api.register_service("user", getpass.getuser())

It may make sense from a maintenance point of view to make the data/function separation. This way, data can be kept constant which simplifies archiving and visualisation, like passing the entire thing to a database, whereas functionality can be kept free of constants.

Open questions

Is additional services something we need, or does it add complexity? When a plug-in requests a service that isn’t available, when do we throw an error?

def process(self, not_exist):
  pass
  1. Thrown during discovery
  2. Thrown during processing, e.g. in the GUI
  3. Silently skipped; rely on external tool for checking correctness.
# Checking correctness
$ pyblish check select_something.py
Plug-in is valid.

Extending Pyblish
#10

DI Transition Guide

Here are some notes of what is involved in converting your plug-ins to a dependency injection-style of working.

Note that none of this is fixed and is very debatable at the moment, so if you have any concerns or input, now is a good time.

  1. In cases where you have either process_context or process_instance, a simple search-and-replace to process will work fine.
  2. In cases where you have both, see below.
  3. For process() to be called, it must either ask for context and/or instance. If neither is present, process() will not be called at all. See below.
  4. During the transition phase, the distinction is made internally by looking for the existence of a process_context or process_instance method.
  5. If either exist, the plug-in is deemed “old-style” and is processed using the current implementation.
  6. If both process and either process_context or process_instance is present, old-style wins and process will not be called.

I’ll update the list as more things come to mind. So far, updating the entire Napoleon extension took less than a minute and was a matter of a simple search-and-replace, leaving the behaviour unspoiled.

Both process_context and process_instance

The current behaviour of this is for process_context to be processed first, followed by process_instance. This behaviour isn’t possible any more. You can however process both in the same function.

def process(self, context, process):
  # do things

In case you do have both, process_instance will overwrite process_context due to your plug-in being re-written to a it’s Dependency Injection equivalent at run-time.

def process_context(self, context):
  # I will not be called. :(

def process_instance(self, instance):
  # Runs as usual

Old-first

The reason for looking for old-style methods before new-style is because of the newly introduced ability to use __init__. In cases where __init__ is used, and process not being implemented, the plug-in is still deemed new-style as __init__ is assumed to not have been in use.

Empty process()

If neither context nor instance is present in the signature of process(), nothing happens.

I struggled to provide the ability to implement “anonymous” processes, for things that do something unrelated to either the Context nor Instance, but primarily to aid in the initial learning phase.

For example.

class MyPlugin(...):
  def process(self):
    cmds.file(exportSelected=True)

This could be a user’s first plug-in. From here, he could learn about the benefits of using Context and thereafter Instance, and thereby learning about why they exist in the first place. Baby-step style.

But, I just can’t for the life of me figure out how to do that in a way that makes sense.

For example, in this case.

class ValidateInstanceA(pyblish.Validator):
  families = ["familyA"]
  def process(self, instance):
    # validate the instance

It’s quite clear that when there isn’t an instance suitable to this family, process should not be called.

However.

class ValidateInstanceA(pyblish.Validator):
  families = ["familyA"]
  def process(self, context):
    # validate the context

What about now? The context isn’t dependent on a family, but should always be called regardless. So clearly, process is called, even if no compatible instance is present.

Which brings us to.

class ValidateInstanceA(pyblish.Validator):
  families = ["familyA"]
  def process(self):
    # do something simple

What happens now? Should it be called?

I considered letting it run if the arguments are either empty, or context is present. But that doesn’t work if other arguments are to be injected.

class ValidateInstanceA(pyblish.Validator):
  families = ["familyA"]
  def process(self, time):
    # do something simple with time

Thoughts?


#11

Hey @mkolar, @BigRoy and @tokejepsen, I just updated the post above, would you mind having a look, specifically the last part about whether or not to run an empty process()?

It’s an subtle but important distinction that will be difficult to change once implemented, your input would be very valuable.


#12

I think this is where a difference is between your and my interpretation.

I would consider that the families attribute here is what limits it from being processed. If this would still get processed even if no compatible instance is available that would only make it more confusing.

If someone really wanted to run just a context check no matter what the family then family can just be ["*"].

As you state the context isn’t dependent on the family. That is true, but the plug-in is dependendent on the family, so should not get processed. :wink:


The other confusing bit here is the amount of time things get run. With context you would expect a single run (over the context) whereas with the instance you want each individual one. It’s a clear distinction that might not only be apparent with dependency injection.

1. Keep process_instance and process_context separate.

Maybe this is where we decide that both process_context() and process_instance() have their respective place. (One gets called once, the other per instance) Of course they could still have the benefits of Dependency Injection for other attributes.

2. Drop the behaviour of per instance processing as a built-in method.

The other side might be to drop the behaviour to run something like process_instance(). Instead only have it run once, always. But that might make it harder to implement behaviour per instance (especially since one error will kill it for each single Instance and you make the plug-in developer responsible for error catching.

I think 1 could work best? It’s already proven itself that it works.


#13

Thanks @BigRoy, really got me thinking.

I woke up this morning to another potential solution - which is to make the presence of instance determine whether or not to process once or per-instance. If instance is not requested, it will process once regardless.

  • Process once
class ValidateInstanceA(pyblish.Validator):
  families = ["*"]
  def process(self):
    # validate the world, once
  • Process per-instance
class ValidateInstanceA(pyblish.Validator):
  families = ["*"]
  def process(self, instance):
    # validate each instance

I like the sound of that.

For clarity, let me give some examples.

class ValidateInstanceA(pyblish.Validator):
  families = ["*"]
  def process(self, context):
    # validate context

This would process once per publish, regardless of instances.

class ValidateInstanceA(pyblish.Validator):
  families = ["myFamily"]
  def process(self, context):
    # validate context

Whereas this would process once per process, only if an instance of family "myFamily" is present.

This is rather complex and possibly confusing, but also flexible and how it works currently.

Should we keep this behaviour?


#14

Having implemented the above and run it through the tests, it looks very good.

Currently, every discovered plug-in is processed at least once, with those requesting instance being processed once per available instance and in case there are no compatible instance doesn’t process at all.

instance then acts as a filter, enabling processing of every instance and preventing processing in cases where instances aren’t available. It’s a subtle difference, but I think it is the one that makes most sense.

It also means SimplePlugin now works as-is, without any custom code. It’s been given an order of -1 meaning they will run before anything else, but can of course be given an order explicitly, effectively making them into SVEC in case of having any of their orders set between 0-3.

This isn’t how it will work. It eliminated the use of plug-ins when no instance were present, like stacks that only operate on the context, and SimplePlugin, which doesn’t have any notion of instances.


#15

About this, repair will also see an update to dependency injection, but I’m expecting a deprecation shortly in favour of Actions.

Your current repair_instance will continue to work fine, with the addition of being able to instead implement repair, passing it instance. As with process_*, a simple search-and-replace will suffice.


#16

So a Plug-in with a specific family will always get its process() triggered (even if not one of those families is available as an instance)? In that case I think it should be clarified that family means instance_family and is only a filter for instances.

I would think it’s more convenient to always filter by family (even for ordinary process), except for when family is not filtered (like family = ["*"]). In that case SimplePlugin should still behave as you want since the default is that plug-ins are unfiltered. Maybe to clarify being unfiltered even further the family might be None by default?

Looking forward to a draft Actions implementations, woohoo!


#17

Yeah, that sounds like it would work. I’ll have to double check the logic…


#18

It looks like it does work, all tests pass and your logic is sound.

Considering it’s easier to go from here and back, than it is to go from allowing everything to adding limits, I’ll leave this in for the next release. I also think it makes more sense.

Thanks for spotting this.


#19

Ok, so the logic is essentially this:

  • Asking for instance will limit your plug-in to only process supported instances.
  • Asking for instance when no instances are present, or only instances of unsupported families, the plug-in will never get run. Not even once.
  • All plug-ins process at least once, unless limited to a particular set of families.
class ValidateInEmergency(pyblish.Validator):
  families = ["emergencyFamily"]
  def process(self):
    call_police()

This plug-in will only run if an instance of emergecyFamily is present.


[solved] InstancePlugin not running as Collector?
#20

This looks quite clear and predictable to me. :+1:


#21

As a last-minute modification, I’m considering adding arbitrary arguments to create_instance.

# Before
instance = context.create_instance(name="MyInstance")
instance.set_data("family", "myFamily")
# After
instance = context.create_instance(name="MyInstance", family="myFamily")

Where name is a first positional, required argument, and everything after are arbitrary keyword arguments.

def create_instance(name, **kwargs):
  instance = Instance(name)
  for key, value in kwargs.items():
    instance.set_data(key, value)

Allowing us to do things a bit more succinctly and flexibly.

instance = context.create_instance(
    name="MyInstance",
    family="myFamily",
    data1="more",
    data2="even more")

It’s a non-breaking change, but would be difficult to turn back from. Considering 1.1 is a large adjustment already, I’d say we’d either implement it now or at 2.0.

Thoughts?


#22

I’d go for it. Look like it would make the code tiny bit cleaner, which is always a good thing


#23

Sounds good. Since it’s a non-breaking change I think implementing it right away should be fine. +1


#24

Cool, consider it done.


#25

This is currently implemented.

Just wondering whether this change is there to stay. Both issues seem to be closed on Github for Collector and Integrator. Everyone seems to be on board for Collector. Feedback on Integrator has been limited, but seems to be accepted as a possible option.

At this moment Collector is an alias to Selector and Integrator is an alias to Conformer. Some points to discuss:

  • Will Collector stay? If so will Selector go away?
  • Will Integrator stay? If so will Conformer go away?
  • If Collector stays than it might be good to swap Conformer to Integrator so its name is a bit more separate. Or it might not bother people?
  • If both Collector and Selector stay and/or both Integrator and Conformer stay might it become confusing to users that both are synonyms/aliases. When mixing extensions you might find yourself having both Collectors and Selectors?

I also noticed you explained the selection/collecting part as Collect in the Learning Pyblish by Example topic. I think it makes sense for starters so maybe make Collector the (only?) standard?

I’d be happy to start refactoring Selector to Collector! And I guess I’m on the same boat for Conformer to Integrator.


#26

At the moment, the official description of Selectors are that they are “also known as Collectors”, and the same for Integrators.

My only compelling reason to not replace Selectors and Conformers today is historical, as SVEC has been what they were called since it was born. :frowning: Apart from that it rolls better off the tounge than CVEI.

Other than that, technically and in terms of clarity it makes for a step forward I think and it would probably be a good idea to make the shift 100%.


#27

Personally I don’t mind what we call them. I’ve gotten used to SVEC, but I could also do CVEI. Think the main thing is just to commit completely.


#28

I think so too. CVEI it is, let’s never look back.

If another stack appears in the future, it will be in the form of an extension. Something SVEC should probably have been to begin with.