HiveBrain v1.2.0
Get Started
← Back to all entries
patternpythonMinor

Modelview programming in PyQt4

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
programmingpyqt4modelview

Problem

In my first attempt, I have tried to load images from disk and load it to a QTableView using QAbstractTableModel. I'd like a general code review of this code.

```
import sys
import os

from PyQt4 import QtGui, QtCore

class MyListModel(QtCore.QAbstractTableModel):
def __init__(self, datain, col, thumbRes, parent=None):
""" datain: a list where each item is a row
"""
self._thumbRes = thumbRes
QtCore.QAbstractListModel.__init__(self, parent)
self._listdata = datain
self._col = col
self.pixmap_cache = {}

def colData(self, section, orientation, role):
if role == QtCore.Qt.DisplayRole:
return None

def headerData(self, section, orientation, role):
if role == QtCore.Qt.DisplayRole:
if orientation in [QtCore.Qt.Vertical, QtCore.Qt.Horizontal]:
return None

def rowCount(self, parent=QtCore.QModelIndex()):
return len(self._listdata)

def columnCount(self, parent):
return self._col

def data(self, index, role):
if role == QtCore.Qt.SizeHintRole:
return QtCore.QSize(*self._thumbRes)

if role == QtCore.Qt.TextAlignmentRole:
return QtCore.Qt.AlignCenter

if role == QtCore.Qt.EditRole:
row = index.row()
column = index.column()
try:
fileName = os.path.split(self._listdata[row][column])[-1]
except IndexError:
return
return fileName

if role == QtCore.Qt.ToolTipRole:
row = index.row()
column = index.column()
try:
self.selectonChanged(row,column)
fileName = os.path.split(self._listdata[row][column])[-1]
except IndexError:
return
return QtCore.QString(fileName)

if index.isValid() and role == QtCore.Qt.DecorationRole:
row = index.row()
colu

Solution

Abstract Model

There are a few improvements I could see in your table model class:

  • In data() you could use elif for all of the if-statements past the first. This will save the need to evaluate each if-condition after the first condition that was found to be True



-
Use key in dict notation. You have this line in your data() method:

if self.pixmap_cache.has_key(value) == False:


This can be written simpler and more Pythonic-ly as:

if not value in self.pixmap_cache:


-
In setData() you caste the result from os.path.split() to a str. This is not necessary as os.path.split() returns a tuple containing strings.

-
Finally, in your selectionChanged() function, raise a NotImplementedError instead of passing. Just in case.

Table

-
In your __init__() function, you set the current directory. You have hard-coded some basic user information. Instead, use the function os.path.expanduser. This will expand ~ into the current user's information.

crntDir = os.path.expanduser('~/Pictures/')


-
In your for-loop checking for file extensions, you simply check for phile.endswith('jpg'). This will match anything that ends with jpg, including directories. Add the period and it will be fixed.

-
There is no need to store MyListModel in a variable. Just go ahead and declare it in self.setModel().

General Comments

  • In your convertToTwoDList function, make your variable names more descriptive. They are very ambiguous at the moment.



  • Look over the PEP8 style guide. It will help your code look more Pythonic. A few examples:



use_underscores in variable names instead of camelCase. Keep consistent spacing throughout your code. Overall it looks very good, however there are times where you have double-blank lines in-between methods.

Code Snippets

if self.pixmap_cache.has_key(value) == False:
if not value in self.pixmap_cache:
crntDir = os.path.expanduser('~/Pictures/')

Context

StackExchange Code Review Q#36843, answer score: 2

Revisions (0)

No revisions yet.