Wie "sauber" ist mein Code programmiert? Was kann man verbessern?

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
Marvin93
User
Beiträge: 36
Registriert: Samstag 4. Mai 2019, 15:16

Donnerstag 14. Januar 2021, 15:54

Hallo,

ich habe mit PyQt5 eine kleine GUI programmiert auf der ich mit der Maus eine Zahl schreiben kann, welche dann von einem Convolutional Neural Network klassifiziert wird. Ich habe kein wirkliches Problem damit und es funktioniert alles genauso wie es soll. Ich wollte den Code hier aber einfach mal posten und fragen, ob ein erfahrener Programmierer drüber schauen kann. Es ist kein super großes oder komplexes Programm, aber mich interessiert einfach wie "sauber" der Code programmiert ist bzw. was ich wirklich unbedingt ändern oder für zukünftige Projekte anders angehen sollte. Vielleicht hat ja jemand Zeit und Lust kurz drüberzuschauen und mir ein paar Tipps zu geben. Ich hab das Programm auf 4 Module aufgeteilt.
Eine Sache die mich auch etwas verwundert ist, dass ich mir mit Pyinstaller eine .exe Datei generieren habe, welche über 600 MB groß geworden ist. Das kommt mir doch sehr viel vor für so ein kleines Programm. Woran kann das liegen?

main.py

Code: Alles auswählen

import sys
from PyQt5.QtWidgets import QMessageBox, QMainWindow, QApplication
from mainwindow import Ui_MainWindow
from model_check import Predictor

class MainWindow(QMainWindow):
    def __init__(self, parent=None):
        super().__init__(parent)

        self.ui = Ui_MainWindow()
        self.ui.setupUi(self)
 
        self.ui.Classify.clicked.connect(self.classification)
        self.ui.Clear.clicked.connect(self.ui.Drawfield.clear)

        self.model = Predictor()

    def classification(self):
        self.number = self.ui.Drawfield.get_number()
        self.prediction = self.model.number_prediction(self.number)
        msg = QMessageBox()
        msg.setWindowTitle("Classification")
        msg.setText("The written number was classified as %s" % (self.prediction))
        msg.setDetailedText("This prediction was made by a previously trained convolutional neural network.\nIt was trained on the MNIST Dataset with a extensive data augmentation to make sure that the classification works properly.")
        msg.exec_()
        
def main():
    app = QApplication(sys.argv)

    window = MainWindow()
    window.show()

    sys.exit(app.exec())

if __name__ == "__main__":
    main()
mainwindow.py

Code: Alles auswählen

from PyQt5 import QtCore, QtGui, QtWidgets
from Mycanvas import Canvas


class Ui_MainWindow(object):
    def setupUi(self, MainWindow):
        MainWindow.setObjectName("MainWindow")
        MainWindow.resize(578, 677)
        self.centralwidget = QtWidgets.QWidget(MainWindow)
        self.centralwidget.setObjectName("centralwidget")
        self.gridLayout = QtWidgets.QGridLayout(self.centralwidget)
        self.gridLayout.setObjectName("gridLayout")
        self.Drawfield = Canvas(self.centralwidget)
        self.Drawfield.setMinimumSize(QtCore.QSize(560, 560))
        self.Drawfield.setMaximumSize(QtCore.QSize(560, 560))
        self.Drawfield.setCursor(QtGui.QCursor(QtCore.Qt.CrossCursor))
        self.Drawfield.setMouseTracking(True)
        self.Drawfield.setTabletTracking(True)
        self.Drawfield.setObjectName("Drawfield")
        self.gridLayout.addWidget(self.Drawfield, 2, 0, 1, 1)
        self.Clear = QtWidgets.QPushButton(self.centralwidget)
        self.Clear.setObjectName("Clear")
        self.gridLayout.addWidget(self.Clear, 0, 0, 1, 1)
        self.Classify = QtWidgets.QPushButton(self.centralwidget)
        self.Classify.setObjectName("Classify")
        self.gridLayout.addWidget(self.Classify, 1, 0, 1, 1)
        MainWindow.setCentralWidget(self.centralwidget)
        self.menubar = QtWidgets.QMenuBar(MainWindow)
        self.menubar.setGeometry(QtCore.QRect(0, 0, 578, 21))
        self.menubar.setObjectName("menubar")
        MainWindow.setMenuBar(self.menubar)
        self.statusbar = QtWidgets.QStatusBar(MainWindow)
        self.statusbar.setObjectName("statusbar")
        MainWindow.setStatusBar(self.statusbar)

        self.retranslateUi(MainWindow)
        QtCore.QMetaObject.connectSlotsByName(MainWindow)

    def retranslateUi(self, MainWindow):
        _translate = QtCore.QCoreApplication.translate
        MainWindow.setWindowTitle(_translate("MainWindow", "MainWindow"))
        self.Clear.setText(_translate("MainWindow", "Clear"))
        self.Classify.setText(_translate("MainWindow", "Classify"))
MyCanvas.py

Code: Alles auswählen

from PyQt5.QtWidgets import QWidget
from PyQt5.QtGui import QImage, QPainter, QPen
from PyQt5.QtCore import Qt, QPoint
import numpy as np


class Canvas(QWidget):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.image = QImage(560, 560, QImage.Format_RGB32)
        self.image.fill(Qt.white)

        self.drawing = False
        self.brushSize = 56
        self.brushColor = Qt.black
        self.lastPoint = QPoint()
    
    def mousePressEvent(self, event):
        if event.button() == Qt.LeftButton:
            self.drawing = True
            self.lastPoint = event.pos()

    def mouseMoveEvent(self, event):
        if(event.buttons() & Qt.LeftButton) & self.drawing:
            painter = QPainter(self.image)
            painter.setPen(QPen(self.brushColor, self.brushSize, Qt.SolidLine, Qt.RoundCap, Qt.RoundJoin))
            painter.drawLine(self.lastPoint, event.pos())
            self.lastPoint = event.pos()
            self.update()

    def mouseReleaseEvent(self, event):
        if event.button() == Qt.LeftButton:
            self.drawing = False

    def paintEvent(self, event):
        canvasPainter  = QPainter(self)
        canvasPainter.drawImage(self.rect(),self.image, self.image.rect())

    def get_number(self):
        image = self.convertQImageToMat(self.image)
        gray = self.rgb2gray(image)
        new_img = np.full((28,28), 0)
        for i in range(0, 560, 20):
            new_img[int(i/20)] = gray[i][0::20]
        return(new_img)

    def convertQImageToMat(self, incomingImage):
        incomingImage = incomingImage.convertToFormat(4)

        width = incomingImage.width()
        height = incomingImage.height()

        ptr = incomingImage.bits()
        ptr.setsize(incomingImage.byteCount())
        image_array = np.array(ptr).reshape(height, width, 4)
        return(image_array)

    def rgb2gray(self, rgb):
        gray = np.round(np.dot(rgb[...,:3], [0.1144, 0.587, 0.2989]))
        return(gray)

    def clear(self):
        self.image.fill(Qt.white)
        self.update()
model.py

Code: Alles auswählen

from keras.models import load_model
import numpy as np

class Predictor():
    def __init__(self):
        self.model = load_model('model.h5')

    def img_prep(self, image):
        new_image = 255 - image
        new_image = new_image.reshape(1, new_image.shape[0], new_image.shape[1], 1)
        return(new_image)

    def number_prediction(self, image):
        x_test = self.img_prep(image)
        prediction = np.argmax(self.model.predict(x_test))
        return(prediction)
Thants
User
Beiträge: 32
Registriert: Dienstag 1. Dezember 2020, 12:00

Samstag 16. Januar 2021, 07:30

  • Programm in mehrere Module unterteilt. Das ist gut, es hilft, den Überblick zu behalten und Komponenten evtl. wiederverwenden zu können.
  • Keine "überladenen" Methoden ("Spaghetti-Code"). Das ist auch gut (wobei man von so einem kleinen Programm nicht darauf schließen kann, wie du größere Programme schreiben würdest).
  • Namensgebung ist nicht konsistent.
    • Die Variablen für deine Widgets sind großgeschrieben (Clear, Classify, Drawfield), was normalerweise nur für Klassen verwendet wird.
    • Die Namen sind teilweise Verben, was für Methoden verwendet werden sollte. "Clear" sollte also eher "clearButton" heißen.
    • Manche Variablen sind komplett kleingeschrieben ("centralwidget"), manche benutzen Camel-Case ("gridLayout").
    • Es sieht so aus, als würdest du in deinem eigenen Code zur Python-Konvention tendieren. Da hat man natürlich das Dilemma, dass man mit Qt eine andere Konvention in seinem Programm hat. Da muss man sich eben überlegen, wie man das handhaben will und sich daran halten. In Canvas mischst du aber auch bei deinen eigenen Methoden die Konventionen (convertQImageToMat vs get_number)
    • Methoden-Namen sollten in der Regel Verben sein. "classification" sollte also eher "classify" oder meinetwegen auch "do_classification" heißen.
    • "get_number" finde ich irreführend. Klingt so, als würde die Methode eine Zahl zurückgeben, sie gibt aber ein numpy array zurück.
  • Es gibt im gesamten Programm keinen einzigen Doc-String oder Kommentar. Die Klassen und Methoden sollten dokumentiert sein.
  • Es gibt keine Fehlerbehandlung. Was passiert z.B., wenn die Datei model.h5 nicht existiert? (ich gehe zumindest mal davon aus, dass das ein Dateiname ist. Habe selbst noch nie mit Keras gearbeitet)
  • Predictor: Der Dateiname model.h5 ist direkt im Code eingetragen. Das sollte wohl eher ein Parameter sein, damit man auch andere Modelle laden könnte.
Zur Größe der exe, PyInstaller packt da alles mit rein, was zum Ausführen des Programms benötigt wird (bzw. packt es manchmal auch etwas mehr ein). Ein Batzen ist dabei Qt und bei dir ist eben noch Keras mit drin. Lass PyInstaller mal so laufen, dass er dir ein Verzeichnis mit den gesammelten Daten erstellt, dann kannst du ja mal einen Blick reinwerfen und nachschauen, was da so alles drin ist.
Marvin93
User
Beiträge: 36
Registriert: Samstag 4. Mai 2019, 15:16

Samstag 16. Januar 2021, 14:55

Vielen Dank erstmal für die Mühe. Der Kommentar hilft mir sehr!
Thants hat geschrieben:
Samstag 16. Januar 2021, 07:30
  • Programm in mehrere Module unterteilt. Das ist gut, es hilft, den Überblick zu behalten und Komponenten evtl. wiederverwenden zu können.
  • Keine "überladenen" Methoden ("Spaghetti-Code"). Das ist auch gut (wobei man von so einem kleinen Programm nicht darauf schließen kann, wie du größere Programme schreiben würdest).
Ja, ist recht klein. Bin auch noch an etwas größeren Programmen dran. Vielleicht poste ich die hier auch mal, wenn sie fertig sind. Wollte aber auch keinen zu langen Code posten. Da hat ja keiner Lust sich den anzugucken. Aber freut mich schon mal, dass das soweit in Ordnung ist. Würde mal behaupten, dass das die wichtigsten Punkte sind. Die Namensgebung und Kommentare usw. kann man sich noch leichter angewöhnen als wenn der Code an sich absoluter Mist ist. :D


[*] Namensgebung ist nicht konsistent.
  • Die Variablen für deine Widgets sind großgeschrieben (Clear, Classify, Drawfield), was normalerweise nur für Klassen verwendet wird.
  • Die Namen sind teilweise Verben, was für Methoden verwendet werden sollte. "Clear" sollte also eher "clearButton" heißen.
  • Manche Variablen sind komplett kleingeschrieben ("centralwidget"), manche benutzen Camel-Case ("gridLayout").
  • Es sieht so aus, als würdest du in deinem eigenen Code zur Python-Konvention tendieren. Da hat man natürlich das Dilemma, dass man mit Qt eine andere Konvention in seinem Programm hat. Da muss man sich eben überlegen, wie man das handhaben will und sich daran halten. In Canvas mischst du aber auch bei deinen eigenen Methoden die Konventionen (convertQImageToMat vs get_number)
  • Methoden-Namen sollten in der Regel Verben sein. "classification" sollte also eher "classify" oder meinetwegen auch "do_classification" heißen.
  • "get_number" finde ich irreführend. Klingt so, als würde die Methode eine Zahl zurückgeben, sie gibt aber ein numpy array zurück.
Ja, das mit den Namen ist immer ein Problem. Bin mir da sehr unsicher und generell auch häufig sehr unkreativ :D
Ich habe auch schon gehört, dass sich da sehr viele Leute unsicher sind mit PyQt was die Konvention angeht. Die Funktion convertQImageToMat habe ich ehrlich gesagt so übernommen von irgendwo. Ich erstelle die GUI mit dem qtdesigner der ja auch die Namen so festlegt. Ich nehme an da besteht nicht die Möglichkeit das umzustellen? Das heißt, wenn ich will, dass der ganze Code gleich ist, muss ich wohl die C Konvention verwenden die auch PyQt verwendet. Für mein kleines Ding hier ists ja im Endeffekt nicht so schlimm, aber ist das in der Praxis nicht super problematisch, wenn da mehrere Leute in einem Team zusammenarbeiten? Oder wird da sowas wie PyQt eh nicht verwendet? Ansonsten nur Klassen großschreiben und Verben nur für Methoden usw. habe ich verstanden und werde es nochmal ausbessern :D


  • Es gibt im gesamten Programm keinen einzigen Doc-String oder Kommentar. Die Klassen und Methoden sollten dokumentiert sein.
  • Es gibt keine Fehlerbehandlung. Was passiert z.B., wenn die Datei model.h5 nicht existiert? (ich gehe zumindest mal davon aus, dass das ein Dateiname ist. Habe selbst noch nie mit Keras gearbeitet)
Okay, Kommentare ist klar. Für mich soweit in Ordnung. Wenn ich in einem Jahr oder eine andere Person in den Code guckt, helfen die sehr. Genau, model.h5 ist ein Dateiname und ein paar Exceptions wären sicher sinnvoll. Das stimmt. Werde danach nochmal schauen.


  • Predictor: Der Dateiname model.h5 ist direkt im Code eingetragen. Das sollte wohl eher ein Parameter sein, damit man auch andere Modelle laden könnte.
Da habe ich nochmal eine Frage zu. Wie macht man das am besten? Ich will jetzt auf der GUI nicht unbedingt noch einen Button usw. einfügen, um ein Modell zu laden.

Code: Alles auswählen

class Predictor():
    def __init__(self, model = 'model.h5'):
        self.model = load_model(model)
Wäre sowas schon okay oder wie geht man das am besten an? Wo genau gebe ich an, welches Modell verwendet werden soll?


Zur Größe der exe, PyInstaller packt da alles mit rein, was zum Ausführen des Programms benötigt wird (bzw. packt es manchmal auch etwas mehr ein). Ein Batzen ist dabei Qt und bei dir ist eben noch Keras mit drin. Lass PyInstaller mal so laufen, dass er dir ein Verzeichnis mit den gesammelten Daten erstellt, dann kannst du ja mal einen Blick reinwerfen und nachschauen, was da so alles drin ist.
Das ist ein guter Tipp. Werde ich mir mal genauer anschauen. Dass das etwas größer ist, ist ja okay. Aber 600mb hat mich schon verwundert.
__deets__
User
Beiträge: 9686
Registriert: Mittwoch 14. Oktober 2015, 14:29

Samstag 16. Januar 2021, 15:07

Wenn es unterschiebliche Namenskonventionen gibt, also hier Python und dann Qt (das durch PyQt einfach nur "weitergeleitet" wird), dann beschraenkt man sich eben da auf die Qt-Konvention, wo es notwendig ist. Nichts in *deinem* Code muss also so heissen, sondern kann sich ganz wunderbar an PEP8 halten. Nur die Dinge, die du zB ueberladen hast, und natuerlich die Methoden von Objekten, die du aufrufst - die musst du natuerlich in Qt-Konvention schreiben.

Wie du mit deiner HDF5-Datei umgehen willst, haengt vom Einsatzweck ab. Wenn das ein geschlossenes Programm ist, das "nur" das erkennen mit einem wohldefinierten Netzwerk unterstuezten soll, dann baut man den Code so um, dass er die Datei auch dann findet, wenn man sich nicht zufaellig gerade mal im gleichen Verzeichnis befindet. Indem man sich ausgehend von der __file__-Variable zu der Datei hangelt. Also zB sowas

Code: Alles auswählen

MODEL_PATH = pathlib.Path(__file__).parent / "netzwerk.hdf5"
assert MODEL_PATH.exists()
Benutzeravatar
__blackjack__
User
Beiträge: 8573
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

Samstag 16. Januar 2021, 17:11

@Marvin93: Den ersten Punkt mit dem Aufteilen auf Module, den Thants als positiv anmerkt, würde ich hier eher als negativ anmerken, denn das ist ein Modul pro Klasse. Das macht man in Python eher nicht, denn Module sind dazu da zusammengehörende Konstanten, Funktionen, und Klassen zusammenzufassen. Wenn man grundsätzlich nur eine Klasse im Modul hat, dann nutzt man Module nicht richtig. Das kann mal passieren, dass man nur eine Klasse in einem Modul hat, dann aber nicht weil das Trennungskriterium ”Klasse” hiess. Wenn man Beispielsweise eine Trennlinie zwischen GUI und Programmlogik ziehen würde, dann würde `Predictor` auch alleine in einem Modul landen, aber eben nicht weil es eine Klasse ist, sondern weil es Programmlogik ist und nocht GUI.

Die Namenskonventionen kann man auch als Chance sehen. Es kann helfen GUI und Programmlogik sauber zu trennen in denn in der Programmlogik sollten keine Namen nach Qt-Konventionen stehen.

`Ui_MainWindow` sollte überhaupt nicht im/als Code auftauchen. Man kann *.ui-Dateien zur Laufzeit laden und muss da keinen Code draus generieren. Dann wird das ganze auch ein gutes Stück kleiner und es gibt noch weniger Grund das auf mehrere Module aufzuteilen.

Du hast den Code in `Ui_MainWindow` manuell verändert — davor wird im Kommentar am Anfang der generierten Datei gewarnt. Wenn Du die Datei neu generierst, dann ist diese Änderung wieder weg. Darum macht man das nicht. Und wenn man die *.ui-Datei lädt kann man das ja auch gar nicht.

Die Klasse `Predictor` ist keine Klasse. Wenn man die Funktion `img_prep()` da raus holt, bleibt eine `__init__()` die eigentlich nur ein einziges Argument für die andere Methode bereitsstellt die damit auch einfach nur eine Funktion sein könnte.

Die beiden Funktionen wären auch bedeutent kürzer wenn man nicht jedes kleine Zwischenergebnis an einen Namen binden würde. Das sind Einzeiler.

In der `classification()`-Methode werden zwei neue Attribute eingeführt: das macht man ausserhalb der `__init__()` nicht. Macht hier auch überhaupt gar keinen Sinn. Das sind einfach nur zwei lokale Namen.

Der ``%``-Operator für Zeichenketten ist veraltet.

Ui, ``(event.buttons() & Qt.LeftButton) & self.drawing`` ist ”interessant”. ``&`` als Ersatz für ``and`` zu verwenden kann zu überraschenden Ergebnissen führen wenn man nicht aufpasst. Hier zum Beispiel funktioniert das nur deswegen zufällig weil `Qt.LeftButton` den Wert 1 hat. Mit `Qt::RightButton` würde der Ausdruck beispielsweise nicht mehr funktionieren. Würde ich also als Fehler ansehen.

`convertQImageToMat()` und `rgb2gray()` sind keine Methoden sondern Funktionen die Du in die Klasse gesteckt hast.

Die magische 4 beim `convertToFormat()`-Aufruf sollte man durch die entsprechende Konstante ersetzen. Das sieht der Leser auch gleich in welches Format da konvertiert wird. Die Funktion dürfte eigentlich gar nicht funktinieren weil `reshape()` maximal zwei Argumente erwartet. Da fehlen Klammern um die Shape-Dimensionen.

In der `get_number()`-Methode sind auch ein paar sehr magische Zahlen. Die 560 sollte man sich vom Widget holen und entweder die 20 oder die 28 sollte man ausrechnen. Eventuell sollte man an anderer Stelle auch die 560 ausrechnen. Also falsch beispielsweise die 28 durch das Modell vorgegeben ist, dann sollte das die Basis sein. Und wenn man diese 28 aus dem Model-Objekt bekommen kann, sollte die dort her kommen.

Das sieht auch ein bisschen umständlich aus. Kann man das nicht einfach komplett durch Slicing machen? Ich sehe gerade nicht warum Du das nicht mit einem simplen Ausdruck und ohne ``for``-Schleife und neues leeres Array was dann erst gefüllt wird, machst.

Zwischenstand (ungetestet):

Code: Alles auswählen

#!/usr/bin/env python3
import sys
from pathlib import Path

import numpy as np
from keras.models import load_model
from PyQt5 import uic
from PyQt5.QtCore import QPoint, QSize, Qt
from PyQt5.QtGui import QCursor, QImage, QPainter, QPen
from PyQt5.QtWidgets import QApplication, QMessageBox, QWidget

SELF_PATH = Path(__file__).absolute().parent
MODEL_PATH = SELF_PATH / "netzwerk.hdf5"
assert MODEL_PATH.exists()
UI_PATH = SELF_PATH / "main.ui"

MODEL_IMAGE_SIZE = 28
CANVAS_SIZE_FACTOR = 20
_CANVAS_SIZE = MODEL_IMAGE_SIZE * CANVAS_SIZE_FACTOR
CANVAS_SIZE = QSize(_CANVAS_SIZE, _CANVAS_SIZE)


def prepare_image(image):
    return (255 - image).reshape(1, image.shape[0], image.shape[1], 1)


def get_prediction(model, image):
    return np.argmax(model.predict(prepare_image(image)))


def convert_qimage_to_array(image):
    image = image.convertToFormat(QImage.Format_RGB32)
    ptr = image.bits()
    ptr.setsize(image.byteCount())
    return np.array(ptr).reshape((image.height(), image.width(), 4))


def convert_rgb_to_gray(rgb):
    return np.round(np.dot(rgb[..., :3], [0.1144, 0.587, 0.2989]))


class Canvas(QWidget):
    def __init__(self, *args, **kwargs):
        super().__init__(*args, **kwargs)
        self.image = QImage(CANVAS_SIZE, QImage.Format_RGB32)
        self.image.fill(Qt.white)

        self.drawing = False
        self.brushSize = 56
        self.brushColor = Qt.black
        self.lastPoint = QPoint()

    def mousePressEvent(self, event):
        if event.button() == Qt.LeftButton:
            self.drawing = True
            self.lastPoint = event.pos()

    def mouseMoveEvent(self, event):
        if event.buttons() & Qt.LeftButton and self.drawing:
            painter = QPainter(self.image)
            painter.setPen(
                QPen(
                    self.brushColor,
                    self.brushSize,
                    Qt.SolidLine,
                    Qt.RoundCap,
                    Qt.RoundJoin,
                )
            )
            painter.drawLine(self.lastPoint, event.pos())
            self.lastPoint = event.pos()
            self.update()

    def mouseReleaseEvent(self, event):
        if event.button() == Qt.LeftButton:
            self.drawing = False

    def paintEvent(self, _event):
        QPainter(self).drawImage(self.rect(), self.image, self.image.rect())

    def getImageArray(self):
        image_array = convert_rgb_to_gray(convert_qimage_to_array(self.image))
        return image_array[0::CANVAS_SIZE_FACTOR, 0::CANVAS_SIZE_FACTOR]

    def clear(self):
        self.image.fill(Qt.white)
        self.update()


class MainWindow:
    def __init__(self):
        self.ui = uic.loadUi(str(UI_PATH))

        self.canvas = Canvas(self.ui.centralWidget())
        self.canvas.setMinimumSize(CANVAS_SIZE)
        self.canvas.setMaximumSize(CANVAS_SIZE)
        self.canvas.setCursor(QCursor(Qt.CrossCursor))
        self.canvas.setMouseTracking(True)
        self.canvas.setTabletTracking(True)

        self.ui.classify_button.clicked.connect(self.classify)
        self.ui.clear_button.clicked.connect(self.canvas.clear)

        self.model = load_model(MODEL_PATH)

    def classify(self):
        prediction = get_prediction(self.model, self.canvas.getImageArray())
        message_box = QMessageBox(
            title="Classification",
            text=f"The written number was classified as {prediction}",
        )
        message_box.setDetailedText(
            "This prediction was made by a previously trained convolutional"
            " neural network.\n"
            "It was trained on the MNIST Dataset with a extensive "
            "data augmentation to make sure that the classification"
            " works properly."
        )
        message_box.exec_()


def main():
    app = QApplication(sys.argv)
    window = MainWindow()
    window.ui.show()
    sys.exit(app.exec())


if __name__ == "__main__":
    main()
“Dawn, n.: The time when men of reason go to bed.” — Ambrose Bierce, “The Devil's Dictionary”
Thants
User
Beiträge: 32
Registriert: Dienstag 1. Dezember 2020, 12:00

Sonntag 17. Januar 2021, 00:42

Marvin93 hat geschrieben:
Samstag 16. Januar 2021, 14:55
Ich erstelle die GUI mit dem qtdesigner der ja auch die Namen so festlegt. Ich nehme an da besteht nicht die Möglichkeit das umzustellen?
Die Attributnamen unter denen die Widgets dann in der Klasse auftauchen legst du ja selbst fest (objectName property). Hast du ja offensichtlich auch gemacht, denn QtDesigner vergibt keine Namen mit Großbuchstabe am Anfang.

Die Methodennamen sind natürlich von Qt vorgegeben. mousePressEvent() muss so heißen und kann natürlich nicht einfach in mouse_press_event() umbenannt werden, das wäre ja eine andere Methode.

Für deine eigenen Methoden hast du dann die Wahl, welche Konvention du benutzen willst. PEP8 ist zunächst ja mal nur die Konvention, die in Python für die Standardbibliothek benutzt wird. Ob du das für deinen eigenen Code auch verwenden möchtest ist komplett deine eigene Entscheidung. Wenn es keine weitere Randbedingungen gibt, ist es sinnvoll, sich an PEP8 zu halten, da man es dann insgesamt nur mit einem einzigen Stil zu tun hat und das Programm insgesamt konsistent ist. Es kann aber auch gute Gründe geben, sich gegen PEP8 zu entscheiden (siehe PyQt). PEP8 ist außerdem eine Richtlinie, d.h. es ist kein Gesetz, das in Stein gemeißelt ist, was die Autoren ja auch selbst so anerkennen (siehe Abschnitt "A Foolish Consistency is the Hobgoblin of Little Minds" in PEP8).

Bei solch einem Style-Guide geht es ja darum, Fehler zu vermeiden und den Code lesbarer zu machen. Ich weiß womöglich, dass es eine "get number"-Methode gab, aber wie genau hieß die noch? War es get_number, getnumber, getNumber oder GetNumber? Wenn ich mir das jedesmal neu überlegen oder sogar nachschlagen muss, hält das auf oder führt zu Fehlern (auch wenn eine gute IDE mir die korrekte Schreibweise anzeigen können sollte).

Nimm jetzt mal dein Programm als Beispiel. Das Programm wird von Qt dominiert. Die Standardbibliothek benutzt du so gut wie überhaupt nicht, es kommt noch ein bisschen numpy und keras vor. Die meisten nicht-Qt-Aufrufe bestehen aus einem Wort, denen sieht man die Konvention also nicht an. Wenn ich nichts übersehen habe, gibt es, abgesehen von deinen eigenen Methoden, nur einen einzigen Aufruf, der sich an PEP8 hält, nämlich load_model() von keras. Hättest du dich komplett an den Stil von Qt gehalten, hättest du also eine einzige Inkonsistenz in deinem Programm. Durch deine eigenen PEP8-Methoden, erhöht sich die Inkonsistenz, d.h. dadurch, dass du PEP8 verwendet hast, hast du genau das Gegenteil davon erreicht, was man mit einem Style-Guide eigentlich erreichen möchte.

Will sagen: Python benutzt PEP8 für die Standardbibliothek und viele externe Python-Pakete richten sich ebenfalls danach und das ist auch gut so. Aber: Man muss nicht blind PEP8 folgen, wenn es gute Gründe für einen anderen Stil gibt. Es ist völlig legitim sich bei einem eigenen Projekt für einen anderen Stil (oder gezwungenermaßen einen gemischten Stil) zu entscheiden. Mein Kritikpunkt von oben war ja auch lediglich, dass du bei deinen eigenen Methoden zwei unterschiedliche Stile verwendet hast, das fand ich dann wieder inkonsistent.

In diesem Sinne: "Entweder konsequent oder inkonsequent, aber das ewige hin und her." :)


Marvin93 hat geschrieben:
Samstag 16. Januar 2021, 14:55
Okay, Kommentare ist klar. Für mich soweit in Ordnung. Wenn ich in einem Jahr oder eine andere Person in den Code guckt, helfen die sehr.
Das ist das eine. Aber es kann auch direkt beim Schreiben des Programms dazu führen, dass es robuster wird. Ist es dir schonmal so ergangen, dass du bei irgendeinem Problem nicht weiter wusstest und jemanden um Rat gefragt hast und beim Erklären des Problems ist dir dann die Lösung selbst eingefallen, ohne dass der andere auch nur ein Wort sagen musste? Das Ausformulieren und Niederschreiben eines Sachverhalts hat den gleichen Effekt und dir fallen evtl. Sonderfälle ein, an die du anfangs nicht gedacht hast (so Sachen wie: hm, was passiert denn eigentlich, wenn der Eingabewert 0 oder negativ ist und du daraufhin den Wertebereich prüfst).
Marvin93 hat geschrieben:
Samstag 16. Januar 2021, 14:55
Genau, model.h5 ist ein Dateiname und ein paar Exceptions wären sicher sinnvoll. Das stimmt. Werde danach nochmal schauen.
Vor allem, dass man die Exceptions an irgendeiner Stelle auch mal wieder einfängt und dem Benutzer eine sinnvolle Meldung macht.
Bei Qt gibt es da nochmal gesonderte Fallstricke. Da Slots nicht notwendigerweise direkt ausgeführt werden, hat der Signal-Sender überhaupt nicht die Möglichkeit Exceptions aufzufangen. Wenn in einem Slot eine Exception auftaucht, kann das also das gesamte Programm mit in den Abgrund reißen und du musst dir was überlegen, wie du das in deinem Programm behandeln möchtest.
Marvin93 hat geschrieben:
Samstag 16. Januar 2021, 14:55
Da habe ich nochmal eine Frage zu. Wie macht man das am besten? Ich will jetzt auf der GUI nicht unbedingt noch einen Button usw. einfügen, um ein Modell zu laden.
Deine neue Variante ist das, was ich mir vorgestellt hatte. So könnte man immerhin zwei Predictor-Instanzen mit unterschiedlichen Modellen anlegen (z.B. um die Modelle miteinander vergleichen zu können).

Ob du das jetzt mit in deine GUI einbauen willst, dass der Benutzer das Modell ändern kann, ist ja eine ganz andere Frage.


@blackjack: Den meisten deiner Punkte stimme ich zu, aber doch nicht allen. Du hast die Predictor-Klasse durch eine Funktion ersetzt und damit ein Implementierungsdetail der Klasse herausgelöst und dem Aufrufer aufgebürdet. Bei deiner Variante steckt jetzt Wissen darüber wie die Klassifikation abläuft direkt im MainWindow, obwohl das nicht Aufgabe des MainWindows ist. Das MainWindow muss sich jetzt selbst darum kümmern, ein Keras-Modell zu finden und zu laden. Dagegen könnte Marvin in seinem Programm den Klassifikationsalgorithmus komplett durch was anderes ersetzen und es würde nur die Predictor-Klasse betreffen, das MainWindow müsste dafür überhaupt nicht angefasst werden, weshalb für mich Marvins Variante eindeutig die bessere ist. Ich finde, er hat hier objektorientiertes Programmieren mit dem Encapsulation-Prinzip richtig angewendet, um ein größeres Problem in kleinere, unabhängige Komponenten zu zerlegen.

Zum Aufteilen in Module: Schlägst du etwa vor, sämtlichen GUI-Code in ein einziges Modul zu packen? Meinst du tatsächlich "Module" oder redest du nicht eher von Paketen? Es ist ja richtig, dass nicht zwingenderweise jede Klasse in ein eigenes Modul muss (hat Marvin ja auch nicht gesagt), aber wenn eine Klasse (evtl. mit Hilfsklassen) eine eigenständige Einheit bildet (die man z.B. auch in anderen Programmen direkt so verwenden könnte), finde ich ein eigenes Modul, wie bei Canvas, absolut gerechtfertigt.

Und für die Behauptung, dass man das in Python so nicht macht, hätte ich ein Gegenbeispiel, schau dir das unittest-"Modul" aus der Standardbibliothek an. Dort importiert man zwar alles direkt aus "unittest", aber die einzelnen Klassen (TestCase, TestSuite, TextTestRunner, etc.) sind sehr wohl in eigene Module untergebracht und "unittest" selbst ist als Paket organisiert, was die Namen direkt auf Paket-Ebene verfügbar macht.
Bei einem GUI-Programm kann man das genauso machen. Widget-Klassen wie die Canvas-Klasse sind in Modulen, die zu einem "gui"-Paket zusammengefasst werden können (dass er bei diesem Mini-Programm nicht noch zusätzlich ein Paket angelegt hat, habe ich mal so gelten lassen :) ).
Benutzeravatar
__blackjack__
User
Beiträge: 8573
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

Sonntag 17. Januar 2021, 01:57

@Thants: Ich meine das tatsächlich so mit den Modulen und nicht Packages.

`unittest` aus der Standardbibliothek ist ein schlechtes Beispiel weil das kein Python ist, das ist Java als Python verkleidet. Das ist die JUnit-API kopiert. In Python genau so ein Fremdkörper wie der Kram in `xml.dom`. Schau Dir mal `pytest` an. Da fliegt dann auch dieses ganze unnötige Klassengedöns raus. Wie bei `Predictor`. Ja, man kann sich da 1000 Gedanken machen wie man da zig Sachen flexibler und abstrakter machen kann. Das kann man ja auch machen: Wenn man das braucht. Nicht auf Vorrat. YAGNI. Genau so dass man die Canvas-Klasse in einem anderen Programm verwenden könnte: dieses allgemeine Wiederverwendbarkeitsgequatsche ist nach meiner praktischen Erfahrung nach deutlich überbewertet. Und genau wie übermässige Flexibilität auf Vorrat: Wenn man die Klasse tatsächlich mal irgendwo anders braucht, dann muss man sie ja sowieso aus dem Programm heraus lösen und in eine Bibliothek stecken und das Programm darauf hin anpassen. Dabei ist dann egal ob man die vorher einsam und alleine in ein eigenes Modul weg sperrt oder ob sie Spielkameraden hatte. 🙂

Ich habe jedenfalls gute Erfahrung mit dem KISS-Prinzip gemacht. Ich fange in der Regel mit einem Modul an. Wenn das zu gross wird, erstelle ich ein Package mit dem Code des Moduls in der `__init__.py`. Und von da ausgehend schaue ich dann was sich sinnvoll thematisch in ein oder zwei Module auslagern lässt. Und da ist dann eher selten und zufällig nur eine Klasse alleine drin. Eine Klasse pro Modul ist als wenn man in Java jede Klasse nochmal in eigenes Package stecken würde. Das macht einfach keinen Sinn. Das Modul ist in Python eine Organisationseinheit wie ein Package in Java.
“Dawn, n.: The time when men of reason go to bed.” — Ambrose Bierce, “The Devil's Dictionary”
Marvin93
User
Beiträge: 36
Registriert: Samstag 4. Mai 2019, 15:16

Sonntag 17. Januar 2021, 17:33

Okay, da ist wieder einiges an Feedback dabei. Wie ich sehe sind viele Dinge gar nicht so eindeutig richtig oder falsch, sondern häufig auch einfach persönliche Präferenz.
__deets__ hat geschrieben:
Samstag 16. Januar 2021, 15:07
Wie du mit deiner HDF5-Datei umgehen willst, haengt vom Einsatzweck ab. Wenn das ein geschlossenes Programm ist, das "nur" das erkennen mit einem wohldefinierten Netzwerk unterstuezten soll, dann baut man den Code so um, dass er die Datei auch dann findet, wenn man sich nicht zufaellig gerade mal im gleichen Verzeichnis befindet. Indem man sich ausgehend von der __file__-Variable zu der Datei hangelt. Also zB sowas

Code: Alles auswählen

MODEL_PATH = pathlib.Path(__file__).parent / "netzwerk.hdf5"
assert MODEL_PATH.exists()
Ah, ja das ist definitiv sinnvoll.


__blackjack__ hat geschrieben:
Samstag 16. Januar 2021, 17:11
@Marvin93: Den ersten Punkt mit dem Aufteilen auf Module, den Thants als positiv anmerkt, würde ich hier eher als negativ anmerken, denn das ist ein Modul pro Klasse. Das macht man in Python eher nicht, denn Module sind dazu da zusammengehörende Konstanten, Funktionen, und Klassen zusammenzufassen. Wenn man grundsätzlich nur eine Klasse im Modul hat, dann nutzt man Module nicht richtig. Das kann mal passieren, dass man nur eine Klasse in einem Modul hat, dann aber nicht weil das Trennungskriterium ”Klasse” hiess. Wenn man Beispielsweise eine Trennlinie zwischen GUI und Programmlogik ziehen würde, dann würde `Predictor` auch alleine in einem Modul landen, aber eben nicht weil es eine Klasse ist, sondern weil es Programmlogik ist und nocht GUI.
Okay, verstehe. Mein Gedankengang war auch nicht die Trennung nach Klassen. Das ist tatsächlich jetzt Zufall. Meine Trennung war main.py als Programmlogik, dann MyCanvas.py und mainwindow.py als GUI. Das hätte ich am lieben zusammengefasst, aber geht ja nur qtdesigner nicht, weil das ganze jedes mal wieder überschrieben wird. Und dann das Modell auch nochmal in einem dritten Modul. Das war zumindest so mein Gedankengang. Das ganze dient ja der Übersichtlichkeit die ich in diesem kleinem Programm natürlich jetzt nicht unbedingt brauche, weil es sowieso sehr klein ist.


__blackjack__ hat geschrieben:
Samstag 16. Januar 2021, 17:11
`Ui_MainWindow` sollte überhaupt nicht im/als Code auftauchen. Man kann *.ui-Dateien zur Laufzeit laden und muss da keinen Code draus generieren. Dann wird das ganze auch ein gutes Stück kleiner und es gibt noch weniger Grund das auf mehrere Module aufzuteilen.

Du hast den Code in `Ui_MainWindow` manuell verändert — davor wird im Kommentar am Anfang der generierten Datei gewarnt. Wenn Du die Datei neu generierst, dann ist diese Änderung wieder weg. Darum macht man das nicht. Und wenn man die *.ui-Datei lädt kann man das ja auch gar nicht.
Nein, also eigentlich habe ich den Code nicht verändert. Ich hab den Kommentar glaube ich nicht mit kopiert, sondern weggelassen als ich hier meinen Code gezeigt habe. Das könnte sein. Ansonsten wenn irgendwas verändert ist, war es ein versehen.
Aber der Punkt ist interessant. Ich habe zu Beginn tatsächlich noch direkt mit der .ui-Datei gearbeitet. Dann habe ich aber irgendwo auf reddit oder stackoverflow oder so gelesen, dass man das nicht macht und man lieber direkt den Code nutzen soll. Deswegen habe ich das dann auch anders gemacht. Ich muss aber auch sagen, dass es mich eh etwas gestört hat, dass ich nicht mal schnell in den Code gucken und den genauen Namen vom Button oder so nachgucken konnte.


__blackjack__ hat geschrieben:
Samstag 16. Januar 2021, 17:11
Die Klasse `Predictor` ist keine Klasse. Wenn man die Funktion `img_prep()` da raus holt, bleibt eine `__init__()` die eigentlich nur ein einziges Argument für die andere Methode bereitsstellt die damit auch einfach nur eine Funktion sein könnte.

Die beiden Funktionen wären auch bedeutent kürzer wenn man nicht jedes kleine Zwischenergebnis an einen Namen binden würde. Das sind Einzeiler.

In der `classification()`-Methode werden zwei neue Attribute eingeführt: das macht man ausserhalb der `__init__()` nicht. Macht hier auch überhaupt gar keinen Sinn. Das sind einfach nur zwei lokale Namen.

Der ``%``-Operator für Zeichenketten ist veraltet.

Ui, ``(event.buttons() & Qt.LeftButton) & self.drawing`` ist ”interessant”. ``&`` als Ersatz für ``and`` zu verwenden kann zu überraschenden Ergebnissen führen wenn man nicht aufpasst. Hier zum Beispiel funktioniert das nur deswegen zufällig weil `Qt.LeftButton` den Wert 1 hat. Mit `Qt::RightButton` würde der Ausdruck beispielsweise nicht mehr funktionieren. Würde ich also als Fehler ansehen.

`convertQImageToMat()` und `rgb2gray()` sind keine Methoden sondern Funktionen die Du in die Klasse gesteckt hast.
Okay, also zusammengefasst passt die Aufteilung doch nicht so ganz. Lieber einzelne Funktionen, wenn eine Klasse nicht nötig ist. Ich glaube ich neige etwas dazu, dass zusammenzufassen in einer Klasse, weil das für mich thematisch zusammen passt. Macht aber wohl aus programmiertechnischer Sicht wenig Sinn.

Nochmal speziell zu den neuen Attributen. Ich verstehe, dass das hier unnötig ist. Ohne das self. würde es den Zweck ja genauso erfüllen. Aber ist es theoretisch möglich, wenn ich das Attribut denn woanders auch noch brauchen würde, sowas wie self.number = None in die __init__ zu schreiben und das hier dann mit dem tatsächlichen Wert zu überschreiben? Also nur hypothetisch. In diesem Fall macht es ja keinen Sinn. Das sehe ich jetzt auch.


__blackjack__ hat geschrieben:
Samstag 16. Januar 2021, 17:11
Die magische 4 beim `convertToFormat()`-Aufruf sollte man durch die entsprechende Konstante ersetzen. Das sieht der Leser auch gleich in welches Format da konvertiert wird. Die Funktion dürfte eigentlich gar nicht funktinieren weil `reshape()` maximal zwei Argumente erwartet. Da fehlen Klammern um die Shape-Dimensionen.

In der `get_number()`-Methode sind auch ein paar sehr magische Zahlen. Die 560 sollte man sich vom Widget holen und entweder die 20 oder die 28 sollte man ausrechnen. Eventuell sollte man an anderer Stelle auch die 560 ausrechnen. Also falsch beispielsweise die 28 durch das Modell vorgegeben ist, dann sollte das die Basis sein. Und wenn man diese 28 aus dem Model-Objekt bekommen kann, sollte die dort her kommen.

Das sieht auch ein bisschen umständlich aus. Kann man das nicht einfach komplett durch Slicing machen? Ich sehe gerade nicht warum Du das nicht mit einem simplen Ausdruck und ohne ``for``-Schleife und neues leeres Array was dann erst gefüllt wird, machst.
Ja diese Zahlen kommen einfach aus dem Format der Bilder des MNIST Datasets (28x28) und dann eben einfach um das 20 fache auf 560 hochskaliert, einfach damit man auf der GUI vernünftig zeichnen kann. Das Ergebnis muss dann später natürlich wieder runterskaliert werden. Aber ja diese Zahlen sind natürlich schwer nachzuvollziehen. Die Funktion habe ich aber auch fast so 1 zu 1 von irgendwo übernommen. Das war das erste mal, dass ich überhaupt mit PyQt5 gearbeitet habe. In dem Code den du gepostet hast, hast du diese Dinge ja global definiert. Wann genau ist es okay das zu tun? Ich dachte immer von allem was ich so gelesen habe, dass man es wenn irgendwie möglich immer vermeiden sollte Dinge global zu definieren.

Das geht sicher mit Slicing irgendwie. Ich habe das sogar probiert, es aber nicht direkt hingekriegt und deswegen eine for Schleife verwendet. Werde aber nochmal schauen, ob ich das etwas eleganter hinbekomme.

Und vielen Dank für den Code. Hilft mir sehr das nochmal anders strukturiert zu sehen, um das ganze besser zu verstehen.



Thants hat geschrieben:
Sonntag 17. Januar 2021, 00:42
Die Attributnamen unter denen die Widgets dann in der Klasse auftauchen legst du ja selbst fest (objectName property). Hast du ja offensichtlich auch gemacht, denn QtDesigner vergibt keine Namen mit Großbuchstabe am Anfang.

Die Methodennamen sind natürlich von Qt vorgegeben. mousePressEvent() muss so heißen und kann natürlich nicht einfach in mouse_press_event() umbenannt werden, das wäre ja eine andere Methode.

Für deine eigenen Methoden hast du dann die Wahl, welche Konvention du benutzen willst. PEP8 ist zunächst ja mal nur die Konvention, die in Python für die Standardbibliothek benutzt wird. Ob du das für deinen eigenen Code auch verwenden möchtest ist komplett deine eigene Entscheidung. Wenn es keine weitere Randbedingungen gibt, ist es sinnvoll, sich an PEP8 zu halten, da man es dann insgesamt nur mit einem einzigen Stil zu tun hat und das Programm insgesamt konsistent ist. Es kann aber auch gute Gründe geben, sich gegen PEP8 zu entscheiden (siehe PyQt). PEP8 ist außerdem eine Richtlinie, d.h. es ist kein Gesetz, das in Stein gemeißelt ist, was die Autoren ja auch selbst so anerkennen (siehe Abschnitt "A Foolish Consistency is the Hobgoblin of Little Minds" in PEP8).

Bei solch einem Style-Guide geht es ja darum, Fehler zu vermeiden und den Code lesbarer zu machen. Ich weiß womöglich, dass es eine "get number"-Methode gab, aber wie genau hieß die noch? War es get_number, getnumber, getNumber oder GetNumber? Wenn ich mir das jedesmal neu überlegen oder sogar nachschlagen muss, hält das auf oder führt zu Fehlern (auch wenn eine gute IDE mir die korrekte Schreibweise anzeigen können sollte).

Nimm jetzt mal dein Programm als Beispiel. Das Programm wird von Qt dominiert. Die Standardbibliothek benutzt du so gut wie überhaupt nicht, es kommt noch ein bisschen numpy und keras vor. Die meisten nicht-Qt-Aufrufe bestehen aus einem Wort, denen sieht man die Konvention also nicht an. Wenn ich nichts übersehen habe, gibt es, abgesehen von deinen eigenen Methoden, nur einen einzigen Aufruf, der sich an PEP8 hält, nämlich load_model() von keras. Hättest du dich komplett an den Stil von Qt gehalten, hättest du also eine einzige Inkonsistenz in deinem Programm. Durch deine eigenen PEP8-Methoden, erhöht sich die Inkonsistenz, d.h. dadurch, dass du PEP8 verwendet hast, hast du genau das Gegenteil davon erreicht, was man mit einem Style-Guide eigentlich erreichen möchte.

Will sagen: Python benutzt PEP8 für die Standardbibliothek und viele externe Python-Pakete richten sich ebenfalls danach und das ist auch gut so. Aber: Man muss nicht blind PEP8 folgen, wenn es gute Gründe für einen anderen Stil gibt. Es ist völlig legitim sich bei einem eigenen Projekt für einen anderen Stil (oder gezwungenermaßen einen gemischten Stil) zu entscheiden. Mein Kritikpunkt von oben war ja auch lediglich, dass du bei deinen eigenen Methoden zwei unterschiedliche Stile verwendet hast, das fand ich dann wieder inkonsistent.

In diesem Sinne: "Entweder konsequent oder inkonsequent, aber das ewige hin und her." :)
Okay, vielleicht ist das auch nicht das optimale Beispiel, weil der Code ja insgesamt sehr klein ist. Aber ja ich verstehe den Sinn dahinter. Fand auch den Ansatz von __Blackjack__ interessant durch die Namensgebung die GUI und die Programmlogik zu trennen. Also das ist letztendlich eine persönliche Entscheidung, bei der man sich jedoch Gedanken darüber machen sollte das ganze möglichst übersichtlich zu gestalten.


Thants hat geschrieben:
Sonntag 17. Januar 2021, 00:42
Deine neue Variante ist das, was ich mir vorgestellt hatte. So könnte man immerhin zwei Predictor-Instanzen mit unterschiedlichen Modellen anlegen (z.B. um die Modelle miteinander vergleichen zu können).

Ob du das jetzt mit in deine GUI einbauen willst, dass der Benutzer das Modell ändern kann, ist ja eine ganz andere Frage.
Ja, das klingt sinnvoll. So kann man das Programm leichter erweitern.




Also die beste Aufteilung auf Module scheint wohl nicht so eindeutig zu sein. Der Code ist natürlich sehr klein und vielleicht nicht so repräsentativ für das Thema. Ich verstehe das ganze jetzt so, dass man die Module nicht nach Klassen aufteilen sollte. Zumindest nicht in Python. Sondern eher nach thematischen Zusammenhängen der Funktionen, Klassen usw.
Ich verstehe aber ehrlich gesagt auch den Unterschied zwischen Modulen und Packages nicht so richtig :D
Benutzeravatar
__blackjack__
User
Beiträge: 8573
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

Sonntag 17. Januar 2021, 18:05

@Marvin93: Welche Begründung wurde bei Reddit denn für das generieren von Code genannt? Die GUI sind ja eigentlich Daten und der Schitt Quelltext daraus zu generieren ist ein extra Schritt den man sich sparen kann. Das erspart man dann auch Leuten die den Quelltext bekommen — das die da nicht nur als extra Schritt die Datei generieren müssen, denn generierte Daten gehören in der Regel als redundante Daten nicht in die Versionsverwaltung.

Was Du verändert hast ist `Canvas` und der Import davon. Das kennt der Designer ja nicht.

Attribute bei denen man in der `__init__()` noch keinen sinnvollen Wert hat, kann man dort mit `None` belegen, ja.

Die Grössen für Model/Canvas habe ich vor allem deswegen global definiert weil ich nicht wusste ob man die 28 aus dem Modell abfragen kann. Falls das geht, würde man die sich natürlich besser von dort holen.

Ansonsten ist die Frage wie man den Faktor anders definieren sollte wenn der nicht für den Benutzer irgendwo einstellbar ist. Wenn das eine Konstante ist, dann ist das nun mal eine Konstante, und dafür definiert man Konstanten. 🙂 Damit man a) keine magischen Zahlen im Quelltext hat, und b) die Werte als Programmierer leicht anpassen kann ohne den gesamten Quelltext nach vorkommen der Zahl zu durchsuchen und anpassen zu müssen. Mit der Fehlerquelle das man nicht alle erwischt und/oder auch Zahlen ändert die den gleichen Wert, aber eine andere Bedeutung haben.

Das mit dem Slicing habe ich doch im Code gezeigt: ``some_array[::n, ::m]`` erzeugt ein Array mit jedem m-ten Wert aus jeder n-ten Zeile:

Code: Alles auswählen

In [109]: A                                                                     
Out[109]: 
array([[ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9],
       [10, 11, 12, 13, 14, 15, 16, 17, 18, 19],
       [20, 21, 22, 23, 24, 25, 26, 27, 28, 29],
       [30, 31, 32, 33, 34, 35, 36, 37, 38, 39],
       [40, 41, 42, 43, 44, 45, 46, 47, 48, 49],
       [50, 51, 52, 53, 54, 55, 56, 57, 58, 59],
       [60, 61, 62, 63, 64, 65, 66, 67, 68, 69],
       [70, 71, 72, 73, 74, 75, 76, 77, 78, 79],
       [80, 81, 82, 83, 84, 85, 86, 87, 88, 89],
       [90, 91, 92, 93, 94, 95, 96, 97, 98, 99]])

In [110]: A[::2, ::2]                                                           
Out[110]: 
array([[ 0,  2,  4,  6,  8],
       [20, 22, 24, 26, 28],
       [40, 42, 44, 46, 48],
       [60, 62, 64, 66, 68],
       [80, 82, 84, 86, 88]])
Packages sind eine Möglichkeit mehrere Module (und Unterpackages) zusammen zu fassen. Technisch ist ein Package ein Verzeichnis in dem eine `__init__.py` liegt. In die kommt der Code den man als Modul bekommt wenn man das Package importiert.
“Dawn, n.: The time when men of reason go to bed.” — Ambrose Bierce, “The Devil's Dictionary”
Marvin93
User
Beiträge: 36
Registriert: Samstag 4. Mai 2019, 15:16

Sonntag 17. Januar 2021, 19:33

__blackjack__ hat geschrieben:
Sonntag 17. Januar 2021, 18:05
@Marvin93: Welche Begründung wurde bei Reddit denn für das generieren von Code genannt? Die GUI sind ja eigentlich Daten und der Schitt Quelltext daraus zu generieren ist ein extra Schritt den man sich sparen kann. Das erspart man dann auch Leuten die den Quelltext bekommen — das die da nicht nur als extra Schritt die Datei generieren müssen, denn generierte Daten gehören in der Regel als redundante Daten nicht in die Versionsverwaltung.
Kann ich ehrlich gesagt gar nicht mehr sagen :D Ich habe das nur so gelesen und habs übernommen. Hätte das vielleicht nochmal hinterfragen müssen. Deine Begründung macht schon Sinn für mich.


__blackjack__ hat geschrieben:
Sonntag 17. Januar 2021, 18:05
Was Du verändert hast ist `Canvas` und der Import davon. Das kennt der Designer ja nicht.
Ja, also man kann den Import und Canvas auch im qtdesigner erstellen. Wenn man mit rechtsklick auf das Widget geht kann man dafür eine benutzerdefinierte Klasse definieren, welche dann auch so benannt und automatisch importiert wird. Ich habe den Import Befehl nur an den Anfang gesetzt, weil ich das übersichtlicher fand. Normalerweise wird der ganz am Ende des Codes angehängt. Das heißt, wenn ich jetzt die GUI nochmal neu exportiere, sieht der Code genauso aus. Nur halt mit dem Import ganz am Ende des Codes und dieser Warnung am Anfang.

__blackjack__ hat geschrieben:
Sonntag 17. Januar 2021, 18:05
Die Grössen für Model/Canvas habe ich vor allem deswegen global definiert weil ich nicht wusste ob man die 28 aus dem Modell abfragen kann. Falls das geht, würde man die sich natürlich besser von dort holen.

Ansonsten ist die Frage wie man den Faktor anders definieren sollte wenn der nicht für den Benutzer irgendwo einstellbar ist. Wenn das eine Konstante ist, dann ist das nun mal eine Konstante, und dafür definiert man Konstanten. 🙂 Damit man a) keine magischen Zahlen im Quelltext hat, und b) die Werte als Programmierer leicht anpassen kann ohne den gesamten Quelltext nach vorkommen der Zahl zu durchsuchen und anpassen zu müssen. Mit der Fehlerquelle das man nicht alle erwischt und/oder auch Zahlen ändert die den gleichen Wert, aber eine andere Bedeutung haben.
Ja, das verstehe ich. Macht Sinn. Theoretisch könnte ich die 28 glaube ich aus der model.h5 Datei bekommen. Bin mir da aber nicht sicher. Das könnte ich aber nochmal ausprobieren.


__blackjack__ hat geschrieben:
Sonntag 17. Januar 2021, 18:05
Das mit dem Slicing habe ich doch im Code gezeigt: ``some_array[::n, ::m]`` erzeugt ein Array mit jedem m-ten Wert aus jeder n-ten Zeile:

Code: Alles auswählen

In [109]: A                                                                     
Out[109]: 
array([[ 0,  1,  2,  3,  4,  5,  6,  7,  8,  9],
       [10, 11, 12, 13, 14, 15, 16, 17, 18, 19],
       [20, 21, 22, 23, 24, 25, 26, 27, 28, 29],
       [30, 31, 32, 33, 34, 35, 36, 37, 38, 39],
       [40, 41, 42, 43, 44, 45, 46, 47, 48, 49],
       [50, 51, 52, 53, 54, 55, 56, 57, 58, 59],
       [60, 61, 62, 63, 64, 65, 66, 67, 68, 69],
       [70, 71, 72, 73, 74, 75, 76, 77, 78, 79],
       [80, 81, 82, 83, 84, 85, 86, 87, 88, 89],
       [90, 91, 92, 93, 94, 95, 96, 97, 98, 99]])

In [110]: A[::2, ::2]                                                           
Out[110]: 
array([[ 0,  2,  4,  6,  8],
       [20, 22, 24, 26, 28],
       [40, 42, 44, 46, 48],
       [60, 62, 64, 66, 68],
       [80, 82, 84, 86, 88]])
Richtig, das habe ich übersehen. Ich habe den Code auch noch nicht ausprobiert, sondern nur im Forum hier angeguckt. Hm, ich war mir sicher, dass ich das sogar so ausprobiert habe. Hatte aber anscheinend wohl noch irgendeinen Fehler drin. Das ist auf jeden Fall die elegantere Lösung.

__blackjack__ hat geschrieben:
Sonntag 17. Januar 2021, 18:05
Packages sind eine Möglichkeit mehrere Module (und Unterpackages) zusammen zu fassen. Technisch ist ein Package ein Verzeichnis in dem eine `__init__.py` liegt. In die kommt der Code den man als Modul bekommt wenn man das Package importiert.
Okay. Einfach nur eine weitere Aufteilung für den Fall, dass man sehr viele Module hat.
Antworten