Hey Marcus,
Just a list of feedback as I read through it:
* The quickstart showed 4 different plugin classes that were used. It would be nice if you first just quickly said something like "pyblish has a number of task oriented classes to derive from. these 4 do xyz". Just a short sentence on the purpose of each one.
* In the quick start, you might not want to use an "assert" as an official example of a validator implementation. I believe assert statements can be optimised away from code if the code is run under a python optimisation flag. Might be an edge case, but people may just take your example and always do it. Is there another way to fail the validation? Is it only AssertionError? Or is it any exception raised? Might be good to be absolutely clear about what is expected to communicate a validation failure that will be properly handled by the framework.
* Files - can PYBLISHPLUGINPATH take multiple paths? I. e. ":" or ";" separated?
* Branching 2 - I actually had to read that example a couple times to get the fact that PrintAssets operates on each asset. I know you mentioned earlier that it uses dependency injection, but I have rarely (if ever) seen it used like this in a dynamic language. Normally it is much clearer in a statically typed language where you can read the exact type, as opposed to how this is magical based on the attribute names. The part I found confusing is that not only is it based purely on magic variable names, but in addition to injecting the dependencies, it also changes the behaviour of the function. It was hard for me to get that from reading the code. Two plugins derived from the same plugin base class, implementing the same process() function, with the same number of arguments... but different argument names... will behave differently. What happens if I do process(self, context, asset)? That is legal right? Are there any illegal combinations that will be ambiguous as to what the plugin will actually want to do?
* Validating 1/2 - More use of "assert" as an official example, which might not be a good idea:
https://wiki.python.org/moin/UsingAssertionsEffectively
"Assertions should *not* be used to test for failure cases that can occur because of bad user input or operating system/environment failures, such as a file not being found. Instead, you should raise an exception, or print an error message, or whatever is appropriate. One important reason why assertions should only be used for self-tests of the program is that assertions can be disabled at compile time."
* Report 4 - "Let's get cackin'." typo?
Hope that feedback is useful!
Justin
--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/CAFRtmOCsh6GHa-2gnVkr5qyA2LYTCv%3DpxnZYfUNVL3efhiEqnQ%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA21QavJTO%3D4Ykgbq_-fgMQNa5VjkOEyQRHhY36W%2B%2BQUig%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.
Re: The quickstart showed 4 different plugin classes that were used. It would be nice if you first just quickly said something like “pyblish has a number of task oriented classes to derive from. these 4 do xyz”. Just a short sentence on the purpose of each one.
I agree, done.
Re: Files - can PYBLISHPLUGINPATH take multiple paths? I. e. “:” or “;” separated?
Yes. Added separators to example for clarity.
Report 4 - “Let’s get cackin’.” typo?
Well spotted, fixed.
Re: asserts
Assertions are voluntary and any exception will do.
I use asserts because I look at validations as tests, and I write tests with assertions. They also make for slightly less typing and a more easily read syntax.
For example.
# Before
if not is_correct:
raise Exception("An explanation")
# After
assert is_correct, "An explanation"
In addition, Pyblish isn’t treating the type of exception differently, so having a type in there at all can be confusing. Also exceptions can’t be handled, which is another reason I think asserts are a good fit as assertions aren’t meant to be handled.
But as you say, they can get automatically stripped if one were to decided to run their code through python -O my_plugin.py. It’s something to look out for.
Re: process(self, context, asset)
In practice, this hasn’t been an issue. Developers include what is needed and let the run-time handle when and what is run. In edge-cases however, you’re right, and even I get confused sometimes.
The rule is, run as few times as possible, but no less.
class MyPlugin(...):
def process(self, context):
# 1 context, 10 assets, this only runs once
class MyPlugin(...):
def process(self, asset):
# 1 context, 10 assets, this runs 10 times
class MyPlugin(...):
def process(self, context, asset):
# 1 context, 10 assets, this runs 10 times
Technically, the presence of asset acts like a switch, causing process() to run once per available asset. If asset isn’t there, process() runs once, regardless of what else is there.
Whether it’s Pythonic or suitable for a dynamic language I can’t say, but I’m very interested in exploring alternatives. The desire is to provide functionality that allows developers to operate on either the whole scene, or individual parts of a scene and this has so far proven to be the most elegant and easily understood method of achieving that.
But I’m totally inside the box on this matter, if you can think of an alternative I’d be happy to try it out.
Re: The quickstart showed 4 different plugin classes that were used. It would be nice if you first just quickly said something like “pyblish has a number of task oriented classes to derive from. these 4 do xyz”. Just a short sentence on the purpose of each one.
I agree, done.
Re: Files - can PYBLISHPLUGINPATH take multiple paths? I. e. “:” or “;” separated?
Yes. Added separators to example for clarity.
Report 4 - “Let’s get cackin’.” typo?
Well spotted, fixed.
Re: asserts
Assertions are voluntary and any exception will do.
I use asserts because I look at validations as tests, and I write tests with assertions. They also make for slightly less typing and a more easily read syntax.
For example.
# Before if not is_correct: raise Exception("An explanation") # After assert is_correct, "An explanation"In addition, Pyblish isn’t treating the type of exception differently, so having a type in there at all can be confusing. Also exceptions can’t be handled, which is another reason I think asserts are a good fit as assertions aren’t meant to be handled.
But as you say, they can get automatically stripped if one were to decided to run their code through
python -O my_plugin.py. It’s something to look out for.
Re: process(self, context, asset)
In practice, this hasn’t been an issue. Developers include what is needed and let the run-time handle when and what is run. In edge-cases however, you’re right, and even I get confused sometimes.
The rule is, run as few times as possible, but no less.
class MyPlugin(...): def process(self, context): # 1 context, 10 assets, this only runs once class MyPlugin(...): def process(self, asset): # 1 context, 10 assets, this runs 10 times class MyPlugin(...): def process(self, context, asset): # 1 context, 10 assets, this runs 10 timesTechnically, the presence of
assetacts like a switch, causingprocess()to run once per available asset. Ifassetisn’t there,process()runs once, regardless of what else is there.Whether it’s Pythonic or suitable for a dynamic language I can’t say, but I’m very interested in exploring alternatives. The desire is to provide functionality that allows developers to operate on either the whole scene, or individual parts of a scene and this has so far proven to be the most elegant and easily understood method of achieving that.
But I’m totally inside the box on this matter, if you can think of an alternative I’d be happy to try it out.
To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/CAFRtmOAJb3P-164hLesc-PhiLnABwjUwM8Gfqi9wNKLsRyEWpQ%40mail.gmail.com.
If the tutorial just said plainly that the goal is to raise any exception if validation fails, that would be clear enough.
Not sure if you missed it, or if you think the current phrasing isn’t enough, but it does include that any exception is ok.
“We indicate failure by throwing exceptions of any kind, including assertions.”
http://forums.pyblish.com/t/learning-pyblish-by-example/108/15
Do you think there would be any value is splitting up the different types of processing for clarity?
Funnily enough, this is how things started out.
https://github.com/pyblish/pyblish/issues/127
DI got implemented on a provisional basis primarily to reduce the learning curve for beginners; not having to explain the Context and Asset concepts before writing your first plug-in. The hello world example would have been quite a bit more substantial without it, for example.
Having said that, I am definitely open to alternatives.
Here’s some of the current logic to preserve.
Given a plug-in architecture, where identical plug-ins may run in multiple environments, such as Maya and Houdini or standalone, how could this be implemented otherwise?
On Tue, 11 Aug 2015 10:01 PM Marcus Ottosson <konstr...@gmail.com> wrote:
If the tutorial just said plainly that the goal is to raise any exception if validation fails, that would be clear enough.
Not sure if you missed it, or if you think the current phrasing isn’t enough, but it does include that any exception is ok.
“We indicate failure by throwing exceptions of any kind, including assertions.”
http://forums.pyblish.com/t/learning-pyblish-by-example/108/15
Do you think there would be any value is splitting up the different types of processing for clarity?
Missed it! Sorry.
Funnily enough, this is how things started out.https://github.com/pyblish/pyblish/issues/127
DI got implemented on a provisional basis primarily to reduce the learning curve for beginners; not having to explain the Context and Asset concepts before writing your first plug-in. The hello world example would have been quite a bit more substantial without it, for example.
Having said that, I am definitely open to alternatives.Here’s some of the current logic to preserve.
Process the worldProcess sections of the world in isolationProcess sections based on their type
It’s essentially three conflicting requirements that all must align for a plug-in to run.
Given a plug-in architecture, where identical plug-ins may run in multiple environments, such as Maya and Houdini or standalone, how could this be implemented otherwise?
Well my suggestion doesn't seem to break those goals or make it any less generic. It takes the same notion that your Asset is already a first class concept and just makes an explicit implementation hook for it vs a hook that gets processed a different way. Also, if a context is something that gets shared throughout plugins, is there any reason not to make it immediately available as a member attribute on the plugin instance? Is it expensive to generate, if the plugin isn't requesting one? If so, you could always make it a context() call or a computed read only property. These things seem to reduce the need of dependency injection.
--
You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group.
To unsubscribe from this group and stop receiving emails from it, send an email to python_inside_m...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/CAFRtmOANWQALHbVppMzpuP%3DJ6oHanfzVtGEMLCmM6t9d85iC6g%40mail.gmail.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA0f6SULK5iaNYjauPjWgvPmxSGgyd46JARF1KM3V_3r2A%40mail.gmail.com.