Reuse SelectInvalidNodes action in multiple plugins

Hi!
I am starting to work with pyblish. I have two Validators, ValidateNamingConvention and ValidateUniqueNames.
I have an Action, SelectInvalidNodes that selects the problematic nodes in maya, as shown in the documentation with a little twist.

class SelectInvalidNodes(pyblish.api.Action):
    label = "Select invalid nodes"
    on = "failed"
    icon = "hand-o-up"
    
    def process(self, context, plugin):
        import pymel.core as pm
        self.log.info("Finding bad nodes..")
        nodes = []
        for result in context.data["results"]:
            if result["error"]:
                instance = result["instance"]
                nodes.extend(instance.data['invalidNodes'])
                
        pm.select(nodes)

In the validators, I set the invalid nodes as instance.data['invalidNodes'] = invalid.
The problem is, it is not plugin dependent, so when I select the Action on one of the failed Validators, the invalid nodes from the plugin that was run last get selected (not the one I clicked), because it overwrites the instance.data['invalidNodes'].
I can overcome this error, if I set them in the data, by some unique, plugin dependent name, but it doesn’t seem right.
What is the best way to do it?

You could do this in a single data entry if invalidNodes contains a dictionary where it has invalid nodes stored per plug-in name.

# pseudocode
instance.data["invalidNodes"] = instance.data.get("invalidNodes", dict())
instance.data["invalidNodes"][self.__class__.__name__] = invalid

I’ve been doing something similar but haven’t been storing them in data, but allowed to Actions to trigger a get_invalid method on the plug-ins themselves. The only odd thing about that is that the methods on the Plug-ins must be defined as classmethod or staticmethod because the plugin given to pyblish.api.Action.process() is not an instance but the class type.

class SelectInvalidAction(pyblish.api.Action):
    """Select invalid nodes in Maya when plug-in failed.

    To retrieve the invalid nodes this assumes a static `get_invalid()`
    method is available on the plugin.

    """
    label = "Select invalid"
    on = "failed"  # This action is only available on a failed plug-in
    icon = "search"  # Icon from Awesome Icon

    def process(self, context, plugin):
        from maya import cmds

        errored_instances = _get_errored_instances_from_context(context)

        # Apply pyblish.logic to get the instances for the plug-in
        instances = pyblish.api.instances_by_plugin(errored_instances, plugin)

        # Get the invalid nodes for the plug-ins
        self.log.info("Finding invalid nodes..")
        invalid = list()
        for instance in instances:
            invalid_nodes = plugin.get_invalid(instance)
            if invalid_nodes:
                if isinstance(invalid_nodes, (list, tuple)):
                    invalid.extend(invalid_nodes)
                else:
                    self.log.warning("Plug-in returned to be invalid, "
                                     "but has no selectable nodes.")

        # Ensure unique (process each node only once)
        invalid = list(set(invalid))

        if invalid:
            self.log.info("Selecting invalid nodes: %s" % ", ".join(invalid))
            cmds.select(invalid, r=1, noExpand=True)
        else:
            self.log.info("No invalid nodes found.")
            cmds.select(deselect=True)

Source: https://github.com/BigRoy/pyblish-magenta/blob/master/pyblish_magenta/action.py#L47

A usage can be seen (amongst others) in the following plug-ins:

I made them classmethod whenever I needed access to the class’s variables in the code itself (or more importantly) whenever I wanted it to log something using the Plug-ins logger.

Yes, that is better.

Yeah, that kinda works too, but it means, you have to do the processing twice, which is ok, as long as you have a few nodes, and quick validation process, but if it is heavier, you don’t want to do it more than once. So saving the invalid nodes would be a better idea.

I am still not convinced by these two methods. And just to throw in a third, obvious, but dirty one, I can copy the Action for each plug-in.

Sure. For us it was also a tool for quick continuous debugging. This would allow the artist to fix some things in the scene, and trigger the action again to see if now things might have been fixed instead of collecting and revalidating all over again. :wink: In that case the fixed items wouldn’t be selected anymore. Which in our case then was a faster workflow.

a bit off-topic, but why isn’t re-running specific plug-ins a thing? Can we summon @marcus?

It could be! It just hasn’t been written yet. If you need help submitting a PR for it, let me know.

but why isn’t re-running specific plug-ins a thing?

It’s not merely dependent on a single plug-in in most cases. For example if you’ve made scene changes you might require it to process the Collectors again, otherwise you’re still validating old data. As such the only way to validate a hundred percent correctly is to trigger the full order of plug-ins again.

Giving the artist that much “manual” control might make things behave unexpectedly?

That’s true.

Maybe it could be useful in cases where you can re-run a particular collector as well, before running validation. Updating the context and instances therein.

It would however add to the burden of the developer, to ensure that his collectors are re-entrant, that is, that there are no side-effects from running a collector twice, such as re-adding instances when it should have updated existing ones.

An interesting problem.