PySide interface for Autodesk Maya

Question

I have designed a PySide interface for the 3D rendering software package Maya whose purpose is to take a file with animation on a single stream and break it up and export each range of animation. I'm pretty new to PySide and would like just a general overview (or harsh critique) of what I could do to improve the quality of this code. It's not entirely finished so some features of the UI don't actually hook up to anything yet such as the tpose box.

from PySide import QtGui, QtCore
from maya import OpenMayaUI as omui
from shiboken import wrapInstance
import pymel.core as pm
import os
def mayaMainWindow():
 mainWinPtr = omui.MQtUtil.mainWindow()
 return wrapInstance(long(mainWinPtr), QtGui.QWidget)
class ExportBar(QtGui.QFrame):
 def __init__(self, parent=None):
 super(ExportBar, self).__init__(parent)
 self.parent = parent
 print parent.width(), parent.height()
 self.resize(parent.width() - 30, 75)
 self.widgets = [
 QtGui.QLineEdit(),
 QtGui.QSpinBox(),
 QtGui.QLabel("-"),
 QtGui.QSpinBox(),
 QtGui.QPushButton(" Export "),
 QtGui.QPushButton("Del")
 ]
 self.widgets[1].setMaximum(pm.playbackOptions(max=1, q=1))
 self.widgets[3].setMaximum(pm.playbackOptions(max=1, q=1))
 self.setFrameStyle(QtGui.QFrame.Panel | QtGui.QFrame.Raised)
 self.setLineWidth(2)
 self.initUI()
 def initUI(self):
 mainLO = QtGui.QHBoxLayout()
 self.setLayout(mainLO)
 self.widgets[0].setPlaceholderText("File Name")
 self.widgets[-2].clicked.connect(lambda: exportAnimation(self))
 self.widgets[-1].clicked.connect(self.deleteWidget)
 for w in self.widgets:
 mainLO.addWidget(w)
 def deleteWidget(self):
 self.parent.exportBars.remove(self)
 self.close()
 self.parent.holder.resize(self.parent.holder.width(), self.parent.holder.height() - self.height())
 if self.parent.exportBars == []:
 self.parent.cycleRow.newExportBar()
class FileBrowser(QtGui.QWidget):
 def __init__(self, parent=None):
 super(FileBrowser, self).__init__(parent)
 self.parent = parent
 self.resize(parent.width(), 20)
 mainLO = QtGui.QHBoxLayout()
 self.setLayout(mainLO)
 mainLO.setSpacing(10)
 self.pathBar = QtGui.QLineEdit(parent.workScenePath)
 self.pathBar.textChanged.connect(self.changeWorkPath)
 fileButton = QtGui.QPushButton("Browse Files")
 fileButton.clicked.connect(self.changePath)
 mainLO.addWidget(self.pathBar)
 mainLO.addWidget(fileButton)
 def changePath(self):
 newPath = pm.fileDialog2(fileMode=3, okc="Select")
 try:
 self.pathBar.setText(newPath[0])
 except Exception:
 pass
 def changeWorkPath(self):
 self.parent.workScenePath = self.pathBar.text()
class CycleRow(QtGui.QWidget):
 def __init__(self, parent=None):
 super(CycleRow, self).__init__(parent)
 self.resize(parent.width(), 20)
 self.parent = parent
 mainLO = QtGui.QHBoxLayout()
 mainLO.setContentsMargins(0, 0, 0, 0)
 mainLO.setSpacing(25)
 mainLO.addSpacing(2)
 self.setLayout(mainLO)
 self.newExpBarButton = QtGui.QPushButton(" New Cycle List ")
 self.newExpBarButton.clicked.connect(self.newExportBar)
 self.tPoseFrameBox = QtGui.QSpinBox()
 self.tPoseFrameBox.setContentsMargins(0, 0, 70, 0)
 self.tPoseFrameBox.valueChanged.connect(self.changeTPoseFrame)
 expAllButton = QtGui.QPushButton(" Export All ")
 expAllButton.setContentsMargins(0, 0, 0, 0)
 expAllButton.clicked.connect(lambda: exportAllAnimations(parent))
 label = QtGui.QLabel("T Pose Frame")
 label.setContentsMargins(0, 0, 0, 0)
 mainLO.addWidget(label)
 mainLO.addWidget(self.tPoseFrameBox)
 mainLO.addWidget(self.newExpBarButton)
 mainLO.addWidget(expAllButton)
 def newExportBar(self):
 exportBar = ExportBar(self.parent)
 self.parent.holderLO.addWidget(exportBar)
 self.parent.exportBars.append(exportBar)
 self.parent.holder.resize(self.parent.holder.width(), self.parent.holder.height() + exportBar.height())
 def changeTPoseFrame(self):
 self.parent.tPoseFrame = self.tPoseFrameBox.text()
class AnimationCycleSplitter(QtGui.QWidget):
 def __init__(self, parent=mayaMainWindow()):
 super(AnimationCycleSplitter, self).__init__(parent)
 self.workScenePath = os.path.join(pm.workspace.path, "scenes\\")
 self.setWindowFlags(QtCore.Qt.Window)
 self.setGeometry(300, 300, 600, 350)
 self.setWindowTitle("Animation Cycle Splitter")
 self.exportBars = []
 self.initUI()
 def initUI(self):
 mainLO = QtGui.QVBoxLayout()
 mainLO.setSpacing(10)
 self.setLayout(mainLO)
 browser = FileBrowser(self)
 self.holder = QtGui.QWidget()
 self.holder.resize(self.width() - 30, 0)
 self.holderLO = QtGui.QVBoxLayout()
 self.holderLO.setContentsMargins(0, 0, 0, 0)
 self.holder.setLayout(self.holderLO)
 scrollArea = QtGui.QScrollArea()
 scrollArea.setWidget(self.holder)
 self.cycleRow = CycleRow(self)
 self.tPoseFrame = self.cycleRow.tPoseFrameBox.value()
 self.cycleRow.newExportBar()
 mainLO.addWidget(browser)
 mainLO.addWidget(scrollArea)
 mainLO.addWidget(self.cycleRow)
 self.show()
class Keyframe(object):
 def __init__(self, objs):
 self._frame = pm.currentTime()
 self._values = {attr:attr.get() for obj in objs for attr in obj.listAnimatable()}
 self._objs = objs
 def paste(self, frame):
 pm.currentTime(frame)
 for attr in self._values.keys():
 try:
 attr.set(self._values[attr])
 except RuntimeError:
 pass
 pm.setKeyframe(self._objs)
def exportAllAnimations(gui, fileType=".ma"):
 for ebar in gui.exportBars:
 exportAnimation(ebar, fileType=fileType) 
def getKeysForFramerange(beg, end, joints):
 keys = []
 for num, frame in enumerate(range(beg, end)):
 pm.currentTime(frame)
 keys.append(Keyframe(joints))
 return keys
def pasteKeysForFramerange(keys):
 for frame, key in enumerate(keys):
 key.paste(frame)
def exportAnimation(gui, fileType=".ma"):
 pm.currentTime(gui.tPoseFrame)
 joints = pm.ls(type="joint")
 tPose = [Keyframe(joint) for joint in joints]
 pm.select(joints)
 fullPathToFile = os.path.join(gui.parent.workScenePath, gui.widgets[0].text() + fileType)
 beg, end = gui.widgets[1].value(), gui.widgets[3].value()
 keys = getKeysForFramerange(beg, end, joints)
 pm.copyKey()
 pm.cutKey()
 pasteKeysForFramerange(keys)
 pm.currentTime(0)
 pm.exportAll(fullPathToFile, force=1)
 pm.pasteKey()

This is what it creates after I hit the new cycle list button a few times.

Answer 1

I don't know much about Maya (I've only programmed 3ds Max) so this is a "generic" review.

1. There are no docstrings. What do all these classes and methods do? How are they supposed to be used? Imagine someone having to maintain this code when you're no longer working on it. What questions would they want to ask you?

2. Be careful with import X as Y: unless Y is a very well known abbreviation for X (such as np for numpy) then this makes your code harder for people to read and understand. In this case:

   from maya import OpenMayaUI as omui
   

   you only use omui once, so I think that you don't really gain anything by this abbreviation.

3. Where do the numbers (30, 75, 20, etc.) come from? If these are related to things like the heights of the default fonts, it would be better to calculate these by calling QFontInfo.pixelSize() or whatever. This would make it clear what the numbers mean.

4. It seems to me that there ought to a better way to get a wider button than by padding the label with spaces.

5. There seems to be some confusion about the choice of directory separator. The screenshot shows that pm.workspace.path uses forward slashes but you've used os.path.join which uses native (backwards) slashes.

6. You maintain an exportBars list containing the ExportBar instances in holderLO. So every time you change the latter you have to keep the former in sync. Why not use holderLO.children() and avoid the need to maintain two copies of this data?

7. You copy a key and then cut it:

   pm.copyKey()
   pm.cutKey()
   

   but if I understand the documentation rightly, cutKey also copies to the clipboard, so is the copyKey call really necessary?

8. Why do you put some initialization code in __init__ and some in initUI? There doesn't seem to be a principled separation here.

9. The exportAnimation function needs to know all about the internals of the ExportBar class, so surely it ought to be a method on that class. You could then write:

   self.widgets[-2].clicked.connect(self.exportAnimation)
   

10. Writing self.widgets[X] is quite unclear (which button is number 3, again?). It would be clearer to write the initialization code like this:

    export_button = QtGui.QPushButton(" Export ")
    export_button.clicked.connect(self.exportAnimation)
    delete_button = QtGui.QPushButton("Del")
    delete_button.clicked.connect(self.deleteWidget)
    self.widgets = [..., export_button, delete_button]
    

11. The exportAllAnimations function needs to know about the implementation of the AnimationCycleSplitter class, so surely it ought to be a method on that class.

12. The FileBrowser class manages a path that is stored in parent.workScenePath. This seems wrong to me: what if you wanted to attach multiple FileBrowser objects to the same parent? It would be better for the FileBrowser class to store the path in one of its own attributes, and then the AnimationCycleSplitter class can provide a method that fetches the path from the FileBrowser instance.

13. It seems pointless to have keyword arguments like fileType=".ma" on functions like exportAnimation which are attached to buttons in the GUI. There is no way for the user to pass values for these keyword arguments, so what is their purpose? A global variable would be clearer. (Also, fileType ought to be named fileExtension or something similar.)

14. There are a few places where I would appreciate a comment. (i) what is the purpose of the cutKey and pasteKey operations? Is this to preserve the current selection? But won't it clobber the clipboard? (ii) Why might you get a RuntimeError in Keyframe.paste?

• Argiri Kotsaris: @ejrb Thank you both for the reviews they have been very beneficial, sadly I can only accept one however I learned a lot from both of you. I will make the changes you all suggested and post that for another review later.

Answer 2

My PySide is a bit rusty but I can't see anything immediately wrong with your structure. I have noticed a few places you could modify to make the code more readable/easier to manage though.

1. You've manually padded some strings to center them in widgets, don't push buttons center text automatically? If not you should be able to do something like

   button.setStyleSheet("Text-align:center")
   

   If that doesn't work, python strings can be padded using format(). To pad "New Cycle List" with spaces to 42 characters use:

   '{: ^42}'.format('New Cycle List')
   

   See here in the documentation.

2. In ExportBar.__init__, perhaps group your widgets in a dictionary rather than a list. At the moment, you have to refer back to __init__ to see which widget the following code refers to:

   self.widgets[-1].clicked.connect(self.deleteWidget)
   

   Whereas if self.widgets was a dictionary, it would be clearer:

   self.widgets['deleteButton'].clicked.connect(self.deleteWidget)
   

   Of course one drawback is you'll need to iterate through the widgets with

   for widget in self.widgets.values():
   

3. Are you using an IDE which supports tab completion? If you are, make life easier for yourself by initialising widgets to stored variables before grouping them:

   self.deleteButton = QtGui.QPushButton("Del")
   self.exportButton = QtGui.QPushButton({: ^30}.format("Export"))
   ...
   self.widgets = {
    'deleteButton': self.deleteButton,
    'exportButton': self.exportButton,
    ...
   }
   

   In IDEs I've used, tab completion usually fails when accessing an element in a structure. (ie. ['one', 'two'][0].upp + TAB usually doesn't find upper()). Having direct access to widgets in addition to grouping for iteration is handy in this regard.

4. It's generally bad form to initialise/call a potentially mutable object in definitions as you have done in the following:

   class AnimationCycleSplitter(QtGui.QWidget):
    def __init__(self, parent=mayaMainWindow()):
   

   Instead do:

   def __init__(self, parent=None):
    if parent is None:
    parent = mayaMainWindow()
   

   See here if you're unsure why.

5. Don't be afraid to group everything! Continuing from points 2. and 3., it doesn't hurt to group widgets several times. If you get to the stage when you have a dozen buttons, a few more lineEdits etc, make groups for each of these widget types.

   self.widgets = {'deleteButton': self.deleteButton ...}
   self.buttons = {'delete': self.deleteButton, 'save': self.saveButton ...}
   self.lineEdits = {'password': self.passwordLineEdit ...}
   

Summary

In my experience, when using PySide it helps to write everything as verbosely and clearly as possible. Use dictionaries with meaningful keys to group widgets and make sure your code allows tab completion to work!