5
\$\begingroup\$

Coursera has a course for beginning Machine Learning and the code is written in Octave. This is the beginning of a solution to the week 3 exercises but written in python. Instead of a terminal app I'm also trying to learn Pyside2. The actual functionality of the program may seem silly but I hope that some of the things I'm learning in it will translate to other real world problems. My code so far has two files.

DigitData.py

import tensorflow.keras.datasets.mnist as mnist
import numpy as np


class DigitData(object):

    def __init__(self):
        # load the data from data sets
        (x_train, self.y_train), (self.x_test, self.y_test) = mnist.load_data()
        # reshape x to flatten from 28 x 28 matrix to a single
        # row for each example
        row, column, z_column = x_train.shape
        self.x_train = np.reshape(x_train, (row, column * z_column))

    def example(self, index, train=True):
        if train:
            sel_ex = self.x_train[index, :]
        else:
            sel_ex = self.x_test[index, :]

        return np.reshape(sel_ex, (28, 28))

    def label(self, index, train=True):
        if train:
            return self.y_train[index]
        else:
            return self.y_test[index]

mlgui.py

import sys
import numpy as np

from PySide2.QtWidgets import (QLabel, QApplication, QVBoxLayout, QDialog, QHBoxLayout,
                               QScrollArea, QWidget, QProgressBar, QPushButton)
from PySide2.QtGui import QImage, QPixmap, qRgb
from PySide2.QtCore import Qt, Signal

from DigitData import DigitData


class DigitLabel(QLabel):

    def __init__(self, digit_matrix):
        super(DigitLabel, self).__init__()
        self.img = QImage(int(28), int(28), QImage.Format(24))
        self.img.fill(125)
        self.setPixmap(QPixmap(self.img))
        square = np.reshape(digit_matrix, (28, 28))
        # TODO: save pixel data in QImage as true grayscale 8 bit
        # TODO: place data into QImage without loop
        # TODO: deal with the j and i flipping fix
        for i in range(28):
            for j in range(28):
                pass
                pixel_intensity = square[j, i]
                color = qRgb(pixel_intensity, pixel_intensity, pixel_intensity)
                self.img.setPixel(i, j, color)
        self.setPixmap(QPixmap(self.img))


class DigitTable(QWidget):

    looped = Signal(int)

    def __init__(self):
        super(DigitTable, self).__init__()
        self.load_flag = None

    def loadDigits(self, digit_data):
        # show some data
        hor_layout = QHBoxLayout()
        for j in range(10):
            vert_layout = QVBoxLayout()
            for i in range(600):
                app.instance().processEvents()
                if self.load_flag == "quit":
                    return 0
                DigitLabel(digit_data.example(i+(j*600)))
                vert_layout.addWidget(DigitLabel(digit_data.example(i+(j*600))))
                self.looped.emit(i+(j*600))
            hor_layout.addLayout(vert_layout)

        # Set dialog layout
        if self.load_flag is None:
            self.setLayout(hor_layout)
            return 1


class DigitDialog(QDialog):

    def __init__(self, parent=None):
        super(DigitDialog, self).__init__(parent)
        # load data
        self.data = DigitData()
        v_layout = QVBoxLayout()
        self.bar = QProgressBar()
        self.button = QPushButton("View Digits")
        self.button.clicked.connect(self.load_digits)
        v_layout.addWidget(self.button)
        v_layout.addWidget(self.bar)
        self.setLayout(v_layout)
        self.setFixedWidth(400)

        # vars used in loading digits
        self.scroll = None
        self.dt = None

    def load_digits(self):
        # setup the button to stop the load if desired
        self.button.setText("Quit Loading")
        self.button.clicked.disconnect()
        self.button.clicked.connect(self.quit_load)

        # load the digits
        self.scroll = QScrollArea()
        self.scroll.setVerticalScrollBarPolicy(Qt.ScrollBarAlwaysOn)
        self.scroll.setHorizontalScrollBarPolicy(Qt.ScrollBarAlwaysOff)
        self.scroll.setWidgetResizable(False)
        self.dt = DigitTable()
        self.dt.looped.connect(self.bar.setValue)
        self.bar.setMinimum(0)
        self.bar.setMaximum(6000)
        success = self.dt.loadDigits(self.data)

        if success:
            # load was not interrupted
            self.scroll.setWidget(self.dt)
            self.scroll.setFixedHeight(400)
            self.layout().addWidget(self.scroll)

            # setup button to remove digits
            self.button.setText("Hide Digits")
            self.button.clicked.disconnect()
            self.button.clicked.connect(self.hide_digits)
            self.bar.setValue(0)
        else:
            # set up button to view digits
            # load was interrupted
            self.dt.hide()
            self.dt.deleteLater()
            self.button.setText("View Digits")
            self.button.clicked.disconnect()
            self.button.clicked.connect(self.load_digits)
            self.bar.setValue(0)


    def hide_digits(self):

        # destroy the scrolling widget
        self.scroll.hide()
        self.scroll.deleteLater()

        # set up button to view digits
        self.button.setText("View Digits")
        self.button.clicked.disconnect()
        self.button.clicked.connect(self.load_digits)

        # set Dialog back to original size
        self.adjustSize()

    def quit_load(self):
        self.dt.load_flag = "quit"


if __name__ == '__main__':
    # Create the Qt Application
    app = QApplication(sys.argv)
    # Create and show the form
    dialog = DigitDialog()
    dialog.show()

    # Run the main Qt loop
    sys.exit(app.exec_())
\$\endgroup\$
3
  • \$\begingroup\$ Can you add a description of the exercise to make it easier to review? Questions should stand on their own. \$\endgroup\$ – Mast Oct 3 '19 at 8:16
  • \$\begingroup\$ @Mast, This code loads a visualization (images) of the digits used in the exercise and isn't part of the solution to the exercise. Perhaps I should have updated what the code does do. That seems moot now, but in the future, I'll keep this in mind. \$\endgroup\$ – Alexander Orman Oct 4 '19 at 20:11
  • \$\begingroup\$ Into the question itself, please. Comments are third-rate citizens on Stack Exchange and may be removed at any time. \$\endgroup\$ – Mast Oct 5 '19 at 6:49
6
\$\begingroup\$

Most of your code looks pretty good, so I'll be highlighting the lines I have thoughts about.

But first, are you using Python 3.x? Because if not, please be aware that core dev support for python 2.x will be dropped at the end of this year. So even though you lack the python 3.x tag, I'll pretend you're using it, since all your code looks python 3.x compatible. A few of these things would make you code unable to run on 2.x - I will mark when this happens, to the best of my knowledge.

class DigitData(object):
# Should be:
class DigitData:

Inheritance from object is implicit in python 3.x only.

(x_train, self.y_train), (self.x_test, self.y_test) = mnist.load_data()
# Should be:
(self.x_train, self.y_train), (self.x_test, self.y_test) = mnist.load_data()

You're using it later.

    def label(self, index, train=True):
        if train:
            return self.y_train[index]
        else:
            return self.y_test[index]

Any function returning from a conditional can be rewritten as:

    def label(self, index, train=True):
        if train:
            return self.y_train[index]
        return self.y_test[index]

Of course, in this specific limited case, you could even one-line it:

    def label(self, index, train=True):
        return (self.y_train if train else self.y_test)[index]

But that depends on if you still think this readable. Opinions may differ - I consider this acceptable, but not everyone will. If you do, you can also apply this to your DigitData.example() method.

super(DigitLabel, self).__init__()
# Can be:
super().__init__()

For python 3.x only.

self.img = QImage(int(28), int(28), QImage.Format(24))

You don't need to cast to ints here, the number 28 already is one. You might also be interested in making these "magic" numbers global constants, since you're using them again a few lines later.

for i in range(28):
    for j in range(28):
        pass
        pixel_intensity = square[j, i]
        color = qRgb(pixel_intensity, pixel_intensity, pixel_intensity)
        self.img.setPixel(i, j, color)

You're absolutely right that it shouldn't be done this way. Since you're already using numpy, consider putting that (or more specifically, it's buffer) behind a QIcon. I'm more of a PyQt guy myself rather than PySide, but I believe PySide should also be capable of this.

def loadDigits(self, digit_data):
    # ...

You're skipping a total of 6000 main loop iterations while executing this one method? That smells a bit like suboptimal solutions. Are you really sure you need 6000 widgets in your layouts here?

I'm going to assume you really do need to - this isn't software engineering stack exchange after all. However, I still reccommend you only pause the loop around 1 in 10 iterations, perhaps even less. Same for it's progress bar signal. Especially after you finish optimizing DigitLabel.__init__a bit more - as often as you build it, you really cannot afford much.

self.dt = DigitTable()

Qt does a lot of useful things with it's parent-child systems. You should really use them to. Add self as an argument here, and in the __init__ of DigitTable, pass it to the super() call.

    def quit_load(self):
        self.dt.load_flag = "quit"

This should be a signal that connects to a slot of self.dt.

In general, when loading a lot of data, you might want to look into offloading that into a another thread. Keep in mind that with Qt, you generally cannot do GUI work anywhere else but the main thread.

\$\endgroup\$

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.