22
\$\begingroup\$

I am making a to-do tree application (Earlybird) in Python that will show checkable tasks/subtasks in a tree view. It includes higher-level groups of tasks grouped into blocks (e.g., Work block, Home block) that do not have checkboxes.

It looks like this:

earlybird screen shot

This is is my first application using Python, and object-oriented programming in general. I am usually a Matlab programmer. Hence, feedback on any level of what I've done would be very helpful. I'm about to put the application on Github, and add a ton of features, so now is the perfect time to throw some dynamite at it.

The application has two main classes:

  1. EarlybirdTree: the core tree view. This is subclassed from QTreeView, and defines basic methods (loading and saving files, adding edited items to the undo stack, etc)..
  2. EarlybirdMain: a simple wrapper for EarlybirdTree objects. This is subclassed from QMainWindow, and allows the user to interact with the tree's methods using menus and toolbars.

The following code includes the above two py files, as well as an earlybird data file (testFile.eb). I store the data in json format. Each main function also adds a separate view of the undostack, just for convenience. Note to keep this from blowing up, I haven't included the functionality for adding/removing items and many other bells and whistles.

earlybirdTree.py

# -*- coding: utf-8 -*-
"""
earlybirdTree.py
    Defines the EarlyBirdTree class, a QTreeView subclass that displays a 
    custom QStandardItemModel as a simple to-do tree. The data is saved
    as a custom json file.
"""

import sys
import os
import json
from PySide import QtGui, QtCore


class StandardItemModel(QtGui.QStandardItemModel):
    '''Items will emit this signal when edited'''
    itemDataChanged = QtCore.Signal(object, object, object, object)


class StandardItem(QtGui.QStandardItem):
    ''''Subclass QStandardItem to reimplement setData to emit itemDataChanged'''
    def setData(self, newValue, role=QtCore.Qt.UserRole + 1):
        #print "setData called with role ", role  #for debugging
        if role == QtCore.Qt.EditRole:
            oldValue = self.data(role)
            QtGui.QStandardItem.setData(self, newValue, role)
            model = self.model()
            if model is not None and oldValue != newValue:
                model.itemDataChanged.emit(self, oldValue, newValue, role)
            return True
        if role == QtCore.Qt.CheckStateRole:
            oldValue = self.data(role)
            QtGui.QStandardItem.setData(self, newValue, role)            
            model = self.model()
            if model is not None and oldValue != newValue:                             
                model.itemDataChanged.emit(self, oldValue, newValue, role)
            return True
        QtGui.QStandardItem.setData(self, newValue, role)


class EarlybirdTree(QtGui.QTreeView):
    '''The earlyBird to do tree view, the core class for the application.'''

    def __init__(self, parent=None, filename = None):
        QtGui.QTreeView.__init__(self, parent=None)
        self.parent = parent
        self.filename = filename
        self.model = StandardItemModel()
        self.rootItem = self.model.invisibleRootItem()        
        self.setModel(self.model)
        self.makeConnections()
        self.undoStack = QtGui.QUndoStack(self)
        self.setStyleSheet("QTreeView::item:hover{background-color:#999966;}")  
        self.headerLabels = ["Task"]
        self.model.setHorizontalHeaderLabels(self.headerLabels) 
        if self.filename:
            self.loadEarlybirdFile(self.filename)

    def makeConnections(self):
        '''Connect all the signals-slots needed.'''
        self.model.itemDataChanged.connect(self.itemDataChangedSlot)

    def itemDataChangedSlot(self, item, oldValue, newValue, role):
        '''Slot used to push changes of existing items onto undoStack'''
        if role == QtCore.Qt.EditRole:
            command = CommandTextEdit(self, item, oldValue, newValue,
                "Text changed from '{0}' to '{1}'".format(oldValue, newValue))
            self.undoStack.push(command)
            return True
        if role == QtCore.Qt.CheckStateRole:
            command = CommandCheckStateChange(self, item, oldValue, newValue, 
                "CheckState changed from '{0}' to '{1}'".format(oldValue, newValue))
            self.undoStack.push(command)
            return True  

    def clearModel(self):
        '''Clears data from model,clearing the view, but repopulates headers/root.
        Used whenever an .eb file is loaded, or newFile method instantiated'''
        self.model.clear()
        self.model.setHorizontalHeaderLabels(self.headerLabels)
        self.rootItem = self.model.invisibleRootItem()   

    def newFile(self):
        '''Creates blank tree'''
        if not self.undoStack.isClean() and self.saveCheck():
            self.saveTodoData()
        self.filename = None
        self.clearModel()
        self.undoStack.clear()

    def closeEvent(self, event):
        '''Typically closeevent is called by a QMainWindow wrapper, 
        but sometimes we do view these guys standalone'''
        if not self.undoStack.isClean() and self.saveCheck():
            self.fileSave()
        self.close()     

    '''
    ***
    Next five methods are part of mechanics for loading .eb files
    ***
    '''
    def loadEarlybirdFile(self, filename = None):
        '''Opens todo tree file (.eb) and populates model with data.'''
        if not self.undoStack.isClean() and self.saveCheck():
            self.saveTodoData() 
        directoryName = os.path.dirname(filename) if filename else "."
        if not filename:
            filename, foo = QtGui.QFileDialog.getOpenFileName(None,
                    "Load earlybird file", directoryName, 
                    "(*.eb)")          
        if filename:
            with open(filename) as f:
                fileData = json.load(f)
            if self.populateModel(fileData, filename):
                self.expandAll()
                self.filename = filename
                self.undoStack.clear()
                return True        
        return False   

    def populateModel(self, fileData, filename):
        '''Verify that top-level items are blocks, and call methods to load data.'''
        if "taskblocks" not in fileData:
            print "Warning: Cannot load {0}.\n"\
                  "Top level must contain taskblocks.".format(filename)
            return False 
        if "tasks" in fileData:
            print "Warning: only reads taskblocks from top level.\n"\
                  "Igorning top-level tasks in {0}.".format(filename)
        taskblockList = fileData["taskblocks"]
        self.clearModel()
        return self.loadTaskblocks(taskblockList)

    def loadTaskblocks(self, taskblockList):  
        '''Load task blocks into the model'''
        for (blockNum, taskblock) in enumerate(taskblockList): 
            blockNameItem = StandardItem(taskblock["blockname"])       
            self.rootItem.appendRow(blockNameItem)
            if "tasks" in taskblock:
                taskList = taskblock["tasks"]
                self.loadTasks(taskList, blockNameItem) 
        return True      

    def loadTasks(self, taskList, parentItem):
        '''Recursively load tasks until we hit a base task (a task w/o any subtasks).'''
        for (taskNum, task) in enumerate(taskList):
            taskNameItem = StandardItem(task["name"])
            taskNameItem.setCheckable(True)
            #print "task and done", task["name"], task["done"]
            if task["done"]:
                taskNameItem.setCheckState(QtCore.Qt.Checked)           
            else:
                taskNameItem.setCheckState(QtCore.Qt.Unchecked)
            parentItem.appendRow(taskNameItem) #add children only to column 0  
            if "tasks" in task:
                subtaskList = task["tasks"]
                return self.loadTasks(subtaskList, taskNameItem) 

    '''
    ****
    Next seven methods are part of the saving mechanics
    ***
    '''
    def saveCheck(self):
        '''If the document has been changed since last clean state, ask if the user
        wants to save the changes.'''
        if QtGui.QMessageBox.question(self,
                "Earlybird save check",
                "Save unsaved changes first?",
                QtGui.QMessageBox.Yes|QtGui.QMessageBox.No) == QtGui.QMessageBox.Yes:
            return True
        else:
            return False

    def saveTodoData(self): 
        '''Save data from the tree in json format'''
        if self.filename:
            dictModel = self.modelToDict()
            with open(self.filename, 'w') as fileToWrite:
                json.dump(dictModel, fileToWrite, indent=2)
        else:
            self.saveTodoDataAs()
        self.undoStack.clear()

    def saveTodoDataAs(self):
        '''Save data in model as...x'''
        dir = os.path.dirname(self.filename) if self.filename is not None else "."
        self.filename, flt = QtGui.QFileDialog.getSaveFileName(None,
                "EarlyBird: Load data file", dir, "EarlyBird data (*.eb)")           
        if self.filename:
            print "Saving: ", self.filename #for debugging
            dictModel = self.modelToDict()
            with open(self.filename, 'w') as fileToWrite:
                json.dump(dictModel, fileToWrite, indent=2)
        self.undoStack.clear()

    def modelToDict(self):  #def modelToDict(self, parentItem = self.rootItem):
        '''Takes model presently in view, and saves all data as dictionary.
        Called by self.saveTodoData() and self.saveTodoDataAs()'''
        dictModel = {}       
        if self.rootItem.rowCount():           
            dictModel["taskblocks"]= self.createTaskblockList(self.rootItem)
            return dictModel

    def createTaskblockList(self, parentItem):
        '''Creates list of task blocks, and their tasks (latter using createTasklist).
        Called by modelToDict which is used to save the model as a dictionary'''
        numChildren = parentItem.rowCount()
        if numChildren:
            taskblockList = [None] * numChildren
            childList = self.getChildren(parentItem)
            for childNum in range(numChildren):
                childItem = childList[childNum]
                childTaskblockData = {}
                childTaskblockData["blockname"]=childItem.text()               
                #now see if the block has children (tasks)
                if childItem.rowCount():
                    childTaskblockData["tasks"] = self.createTaskList(childItem)
                taskblockList[childNum] = childTaskblockData
            return taskblockList
        else:
            return None

    def createTaskList(self, parentItem):
        '''Recursively traverses model creating list of tasks to
        be saved as json'''
        numChildren = parentItem.rowCount()
        if numChildren:
            taskList = [None] * numChildren
            childList = self.getChildren(parentItem)
            for childNum in range(numChildren):
                childItem = childList[childNum]
                childTaskData = {}
                childTaskData["name"] = childItem.text()
                childTaskData["done"] = True if childItem.checkState() else False
                #now see if the present child has children
                if childItem.rowCount():
                    childTaskData["tasks"] = self.createTaskList(childItem)
                taskList[childNum] = childTaskData
            return taskList
        else:
            return None

    def getChildren(self, parentItem):
        '''Returns list of child items of parentItem. Used when converting
        model to dictionary for saving as json'''
        numChildren = parentItem.rowCount()
        if numChildren > 0:
            childItemList = [None] * numChildren
            for childNum in range(numChildren):
                childItemList[childNum] = parentItem.child(childNum, 0)
        else:
            childItemList = None
        return childItemList


class CommandTextEdit(QtGui.QUndoCommand):
    '''Command for undoing/redoing text edit changes, to be placed in undostack'''
    def __init__(self, earlybirdTree, item, oldText, newText, description):
        QtGui.QUndoCommand.__init__(self, description)
        self.item = item
        self.tree = earlybirdTree
        self.oldText = oldText
        self.newText = newText

    def redo(self):      
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot) 
        self.item.setText(self.newText)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 

    def undo(self):
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot) 
        self.item.setText(self.oldText)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 


class CommandCheckStateChange(QtGui.QUndoCommand):
    '''Command for undoing/redoing check state changes, to be placed in undostack'''
    def __init__(self, earlybirdTree, item, oldCheckState, newCheckState, description):
        QtGui.QUndoCommand.__init__(self, description)
        self.item = item
        self.tree = earlybirdTree
        self.oldCheckState = QtCore.Qt.Unchecked if oldCheckState == 0 else QtCore.Qt.Checked
        self.newCheckState = QtCore.Qt.Checked if oldCheckState == 0 else QtCore.Qt.Unchecked

    def redo(self):
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot) 
        self.item.setCheckState(self.newCheckState)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 

    def undo(self):
        self.item.model().itemDataChanged.disconnect(self.tree.itemDataChangedSlot)
        self.item.setCheckState(self.oldCheckState)
        self.item.model().itemDataChanged.connect(self.tree.itemDataChangedSlot) 


def main():
    ebApp = QtGui.QApplication(sys.argv)
    firstEb = EarlybirdTree(filename = "testFile.eb")
    firstEb.show()
    undoView = QtGui.QUndoView(firstEb.undoStack)
    undoView.show()
    sys.exit(ebApp.exec_())


if __name__ == "__main__":
    main()

earlybirdMain.py

# -*- coding: utf-8 -*-
"""
earlybirdMain.py: 
    A wrapper for the EarlybirdTree class (defined in earlybirdTree.py).
    The wrapper allows for simple menu/toolbar-based user interaction
    with an earlybird to do tree. Includes undo/redo functionality.
"""
import sys
import os
from PySide import QtGui, QtCore
from earlybirdTree import EarlybirdTree

class EarlybirdMain(QtGui.QMainWindow):
    '''Main window to wrap an EarlybirdTree'''
    def __init__(self, filename = None):
        QtGui.QMainWindow.__init__(self)
        self.setAttribute(QtCore.Qt.WA_DeleteOnClose) 
        self.view = EarlybirdTree(self, filename) 
        self.model = self.view.model
        self.windowTitleSet()
        self.setCentralWidget(self.view)
        self.createStatusBar()
        self.createActions()
        self.createToolbars()
        self.createMenus()

    def createToolbars(self):
        '''Create toolbars for actions on files and items'''
        self.fileToolbar = self.addToolBar("File actions")
        self.fileToolbar.addAction(self.fileNewAction)
        self.fileToolbar.addAction(self.fileOpenAction)
        self.fileToolbar.addAction(self.fileSaveAction)
        self.fileToolbar.addAction(self.fileSaveAsAction)
        self.itemToolbar = self.addToolBar("Item actions")
        self.itemToolbar.addAction(self.undoAction)
        self.itemToolbar.addAction(self.redoAction)

    def closeEvent(self, event):
        '''If data has been changed, ask user if they want to save it'''
        if not self.view.undoStack.isClean() and self.view.saveCheck():
            self.view.fileSave()
        self.close()

    def createMenus(self):
        '''Create menu for actions on files'''
        self.fileMenu = self.menuBar().addMenu("&File")
        self.fileMenu.addAction(self.fileOpenAction)    
        self.fileMenu.addAction(self.fileNewAction)
        self.fileMenu.addAction(self.fileSaveAction)
        self.fileMenu.addAction(self.fileSaveAsAction)

    def createActions(self):
        '''Create all actions to be used in toolbars/menus: calls createAction()'''
        #File actions
        self.fileNewAction = self.createAction("&New", slot = self.newFile,
                shortcut = QtGui.QKeySequence.New, tip = "New file",
                status = "Create a new file")
        self.fileOpenAction = self.createAction("&Open...", slot = self.fileOpen,
                shortcut = QtGui.QKeySequence.Open, tip = "Open file",
                status = "Open an existing earlybird tree")
        self.fileSaveAction = self.createAction("&Save", slot = self.fileSave,
                shortcut = QtGui.QKeySequence.Save, tip = "Save file",
                status = "Save file")
        self.fileSaveAsAction = self.createAction("Save &As", slot = self.fileSaveAs,
                shortcut = QtGui.QKeySequence.SaveAs, tip = "Save file as", status = "Save file as")
        #Item actions
        self.undoAction = self.createAction("Undo", slot = self.view.undoStack.undo,
               shortcut = QtGui.QKeySequence.Undo, tip = "Undo",
               status = "Undo changes")    
        self.redoAction = self.createAction("Redo", slot = self.view.undoStack.redo,
               shortcut = QtGui.QKeySequence.Redo, tip = "Redo",
               status = "Redo changes")

    def createAction(self, text, slot=None, shortcut=None, 
                     tip=None, status = None):
        '''Function called to create each individual action'''
        action = QtGui.QAction(text, self)
        if shortcut is not None:
            action.setShortcut(shortcut)
        if tip is not None:
            action.setToolTip(tip)
        if status is not None:
            action.setStatusTip(status)
        if slot is not None:
            action.triggered.connect(slot)
        return action 

    def createStatusBar(self):                          
        self.status = self.statusBar()
        self.status.setSizeGripEnabled(False)
        self.status.showMessage("Ready")        

    def fileSaveAs(self):
        self.view.saveTodoDataAs()
        self.windowTitleSet()

    def fileSave(self):
        if self.view.filename:        
            self.view.saveTodoData()
        else:
            self.view.saveTodoDataAs()
            self.windowTitleSet()

    def fileOpen(self):
        '''Load earlybird file from memory.'''
        if self.view.loadEarlybirdFile():
            self.model = self.view.model  
            self.windowTitleSet()
            if self.view.filename:
                filenameNopath = QtCore.QFileInfo(self.view.filename).fileName()
                self.status.showMessage("Opened file: {0}".format(filenameNopath))

    def newFile(self):
        '''Opens new blank earlybird file'''
        self.view.newFile()
        self.windowTitleSet()

    def windowTitleSet(self):
        '''Displays filename as window title, if it exists.'''
        if self.view.filename:
            self.setWindowTitle("Earlybird - {}[*]".format(os.path.basename(self.view.filename)))
        else:
            self.setWindowTitle("Earlybird - <untitled>")


def main():   
    ebApp = QtGui.QApplication(sys.argv)
    mainEb = EarlybirdMain(filename = None)#"simpleTodo.eb"
    mainEb.show()
    undoView = QtGui.QUndoView(mainEb.view.undoStack)
    undoView.show()
    sys.exit(ebApp.exec_())


if __name__ == "__main__":
    main()

testFile.eb

{
  "taskblocks": [
    {
      "tasks": [
        {
          "done": false, 
          "name": "Rake leaves"
        }, 
        {
          "done": true, 
          "name": "Eat dinner"
        }
      ], 
      "blockname": "Home"
    }, 
    {
      "tasks": [
        {
          "done": false, 
          "name": "Analysis"
        }, 
        {
          "tasks": [
            {
              "done": false, 
              "name": "Start Github project"
            }, 
            {
              "done": true, 
              "name": "Write readme.md"
            }, 
            {
              "done": false, 
              "name": "Implement functions"
            }
          ], 
          "done": false, 
          "name": "Graphing project"
        }
      ], 
      "blockname": "Work"
    }
  ]
}

The toughest part of the application to figure out was the undo/redo functionality. I have asked three questions about this at SO before finally settling on the strategy above.

\$\endgroup\$
  • \$\begingroup\$ Are you forced to use Python 2.7 or can you use a more recent version? \$\endgroup\$ – Morwenn Apr 29 '15 at 14:07
  • \$\begingroup\$ I'm allergic to Python 3, but could in theory use it. :) I do a lot of scientific computing, so have been waiting with Python 2 until everything is switched over. \$\endgroup\$ – neuronet Apr 29 '15 at 15:23
4
+50
\$\begingroup\$

From your answer to my comment, I will drop the intended Python 3 paragraph (even though there are some goodies you may like in the most recent versions, such as the new pathlib that supersedes the old os.path module) and go straight to the main point:

Qt Designer is the way to go

Writing user interface code is something that nobody likes to do. You could seriously save a good amount of lines of code by designing your main UI in Qt Designer and using pyuic4 or pyside-uic to generate the boring code so that you can concentrate on the Model and Control part of the code.

Internationalization

While it may not be important, it doesn't cost more than a few self.tr() in order to tell which strings may be translated, even if you don't set up any translation for now. But if one day you want to translate everything, you will be glad not to have to reread your entire project to know which strings need to be translated. That said, many of the strings to be translated are automatically marked as such in the code generated by Qt Designer.

Consistency is the key

When reading the following piece of code:

def __init__(self, parent=None, filename = None):
    QtGui.QTreeView.__init__(self, parent=None)

Two things strike me:

  • Was your intention to forward parent to QTreeView.__init__? If so, I fear that it is not what's being done right here.

  • The way declare default values for parameter in inconsistent style-wise. I bet that it is an oversight though since most of your code seems consistent when it comes to the style. If we follow the PEP8 (I guess we should), we should drop the spaces around the = for default parameters:

    def __init__(self, parent=None, filename=None):
        QtGui.QTreeView.__init__(self, parent)
    
\$\endgroup\$
  • \$\begingroup\$ Thanks for the analysis. My goal with the parent in init is to set the parent of the widget in question, passing ownership to the parent so (for instance) the child automatically closes when I close the parent. Wrt QtDesigner, I strangely enjoy hand-writing my code, though I have friends who make fun of me for it. It would save me time, I'm sure: I have used it for simple things before, and never fell in love with it. Finally, I should definitely review review PEP8, I likely violate it frequently. \$\endgroup\$ – neuronet Apr 29 '15 at 19:27
  • \$\begingroup\$ @neuronet Uh, my bad then, I was pretty sure that the parent issue was a typo .___. I can tell you that you do not violate the PEP8 that frequently anyway :) \$\endgroup\$ – Morwenn Apr 30 '15 at 7:27
  • \$\begingroup\$ Now that you mention it again, I do think I don't need the second parent=None because that is already passed as the default to __init__. \$\endgroup\$ – neuronet Apr 30 '15 at 13:35
2
\$\begingroup\$
   self.fileToolbar = self.addToolBar("File actions")
   self.fileToolbar.addAction(self.fileNewAction)
   self.fileToolbar.addAction(self.fileOpenAction)
   self.fileToolbar.addAction(self.fileSaveAction)
   self.fileToolbar.addAction(self.fileSaveAsAction)
   self.itemToolbar = self.addToolBar("Item actions")
   self.itemToolbar.addAction(self.undoAction)
   self.itemToolbar.addAction(self.redoAction)

.

    self.fileMenu.addAction(self.fileOpenAction)    
    self.fileMenu.addAction(self.fileNewAction)
    self.fileMenu.addAction(self.fileSaveAction)
    self.fileMenu.addAction(self.fileSaveAsAction)

You've got a bunch of repeated code. It would be better to instead, make appropriate data structures, then implicitly loop over them.

\$\endgroup\$
  • 2
    \$\begingroup\$ Generally speaking, it doesn't work that well for UI code unfortunately. Things are generally sufficiently close to be annoying, but sufficiently different to have too many special cases when you try to make some generic code to handle them :/ \$\endgroup\$ – Morwenn Apr 29 '15 at 14:03
  • \$\begingroup\$ Yeah I would think you need like 4+ repeated things before you save any lines with a for action in actions: menu.addAction(action) kind of solution. If you do small batches routinely you could make a helper function, but I often end up preferring staying closer to the library and not overly abstracting UI code. \$\endgroup\$ – flutefreak7 Sep 21 '18 at 4:38

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.