Hi, sorry for the late reply. Busy weekend!

On Friday, 18 July 2014 23:36:52 UTC+2, Justin Israel wrote:
>
> I can offer a couple points of feedback so far, but I didn't get too deep 
> into the too since it has some compatibility issues with python2.6 (Maya 
> 2013).
> If you use the str.format() approach for formatting text, python2.6 
> doesn't support the anonymous fields. That was introduced in python2.7
>
> So in python2.7, this is valid:     "My string says {} 
> {}!".format("Hello", "World")
> But in python2.6 you have to name the fields:   "My string says {0} 
> {1}!".format("Hello", "World")
>

Good to know! I'll fix it so that it works with older versions too. :)
 

> Also, there are a couple button callbacks where you use "mm.eval()", but 
> if someone doesn't have maya.mel imported as mm in their global scope 
> (either having done it manually in the script editor, or in their 
> userSetup.py) then it will produce an error. That could be another 
> candidate for partial, doing something like partial(mm.eval, "my mel 
> command"). Otherwise you have to do the more verbose string approach that 
> has to first import maya.mel: "import maya.mel as mm; mm.eval('my mel 
> command')". I kind of prefer the former, if you have to call mel as a 
> python callback. Or it can call a python method you have set up already 
> that will cleanly call the mel command, like:   self.__prefixHierarchy()
>

Thanks, I never noticed that issue, probably because I have always launched 
the script at least once from the script editor, importing maya.mel in the 
global scope, before trying the shelf button. I fixed it creating three 
methods looking like this:

def launch_prefixHierarchy(self, *args):
        """Launches Maya's built-in command"""
        
        from maya.mel import eval as eval
        
        eval('prefixHierarchy') 

Out of curiosity, why did you add the double underscore in the method 
example? I know it is supposed to tell python that the method is private, 
but I don't see why would someone do that (it is more of a general inquiry, 
not a specific one).

* "New-style" python classes always at least subclass object:   class 
> MyClass(object):
>

Ok, not really sure what is going on here, but I'll do some research! To be 
on the safe side, I'll do as you say and take it from there.

* When testing if a list is empty, instead of allocating a new empty list 
> object each time, just to test if it is equal to the other list, you can 
> simply check if your list is empty using:     if not myList
>

Do you mean that instead of doing

if my_list != []:

I should do

if not my_list

? I guess I stick with the first method as it reminds me of the few things 
I remember about C :P
 

> * It is usually best not to have your class constructor have side effects, 
> such as showing the window. It should have a separate method like show(). 
> That way someone can import and create an instance of the window and show 
> it when they want to. Constructors are for setting up the state of the 
> class.
>

Ok, to put in other words, I can leave the __init__ empty, given that there 
isn't much to initialize, and create a new method that does what the 
__init__ was doing. It seems to work, so I'll do it :)

* The approach where you are using the tool_launcher() method with a string 
> tool name might be cleaner if you made those tools formal functions or 
> methods. There are a couple of places where you test for the literal string 
> names in order to do different logic, which could be bug prone because 
> changing the name in one spot means you have to find and replace all of the 
> literal string references. Maybe you might want to define those once in 
> your class as class constants:
>
> class MyClass(object):
>     ACTION_NAME_1 = "action1"
>     ACTION_NAME_2 = "action2"
>
> Or if you want to make the association between an action and a displayable 
> name, you can register a dictionary in your constructor that maps them:
>
> self._actions = {
>     "Action 1": self.__action1,
>     "Action 2": self.__action2,
> }
>
> Then you won't have to do the 'if actionName == "literal name"' test in 
> multiple places to figure out the action to perform. You can just look up 
> the associated action in the dict:  self._actions[actionName]
>

You're very much right. I think I'll live it as it is for now. If happen to 
change that part of the code I'll fix it properly. 
 

> * rsplit() is a method of a string
> so this:    str.rsplit(str(temp_name), split_char, 1)
> can be written as:    temp_name.rsplit(split_char, 1)
>

Ok, now this is interesting. It was definitely stupid of me to use that 
syntax. I do have a question though. I started with:

str.rsplit(temp_name, split_char, 1)

which gave me this:

TypeError: descriptor 'rsplit' requires a 'str' object but received a 
'unicode'

This is why I managed to make the ugly code even uglier with the 
str(temp_name) type conversion.

Now with the question. Following the error stated above, I assumed that if 
I wanted to avoid the same TypeError, your suggestion

temp_name.rsplit(split_char, 1)

needed to be converted to

str(temp_name).rsplit(split_char, 1)

Needles to say, you were right and it works perfectly without conversion. 
Any idea why it needed the conversion in the first case and not in the 
second?


Well, thanks a lot. You've been really helpful. Do you have anything to say 
about the actual functionality of the whole thing? UI design and stuff like 
that?

Thank you very much!

-- 
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 [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/python_inside_maya/ed3474e4-08f8-4b02-9f7d-2b4cd958d6c0%40googlegroups.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to