Combobox aktualisierung und code optimierung

[majorkeen]

Hallo zusammen,

zu folgendem Code hätte ich folgende Fragen:
1. Wie würde dieser Code aussehen, wenn dies ein Profi schreiben würde(Code Optimierung)?
2. Wie kann ich die PNGs die ich anzeigen möchte aktualisieren lassen? Im Tab1 sowie auch im Tab2

Zu meinen Kenntnissen:
Dies ist mein erster Kontakt mit Python bzw. mit einem GUI Coding oder gar Coding auf einem Betriebssystem. Bislang bin ich nur mit Assembler oder manchmal auch C unterwegs ohne Betriebssystem.

Wenn diese Themen schon anders beschrieben werden, würde ich um Stichworten bitten nach denen ich suchen kann.

Vielen Dank!
Gruß Holgus

Mein configuration.cfg:
[codebox=ini file=Unbenannt.ini][section0]
1 = 3
2 = 2
3 = 1
4 = 0[/code]
Mein Python 3.5.2:

Code: Alles auswählen

from tkinter import PhotoImage
from tkinter import Label
from tkinter import mainloop
from tkinter import ttk
import tkinter
import configparser

root = tkinter.Tk()
root.title('ComboTest')

tab = ttk.Notebook(root)
tab1 = ttk.Frame(tab)
tab2 = ttk.Frame(tab)

tab.add(tab1, text='Tab1')
tab.add(tab2, text='Tab2')
tab.grid(row=0, column=0)

config = configparser.RawConfigParser()

list_combo = ["option1","option2","option3","option4"] 
list_col = ["row1","col1","col2","col3","col4"] 

def config_write(section,name,parameter):
        print ('write config to:',section,name,parameter)
        config.set(section,name,parameter)
        with open('configuration.cfg', 'w') as configfile:
                config.write(configfile)

def config_read(section,name):
        print ('read config from:',section, name)
        config.read('configuration.cfg')
        return (config.get(section,name))
#---Tab1------------------------------------------------------------------
label2= Label(tab1, text='row1')
label2.grid(column=0, row=1)
label3= Label(tab1, text='row2')
label3.grid(column=0, row=2)

x=0
for x in range(5):
        label= Label(tab1, text=list_col[x])
        label.grid(column=x, row=0)

# 1st combobox with picture 
box_1 = ttk.Combobox(tab1, value=list_combo, state='readonly')
box_1.set(list_combo[int(config_read('section0','1'))])
box_1.bind('<<ComboboxSelected>>', lambda evente: config_write('section0','1',box_1.current()))
box_1.grid(column=1, row=1)
img_tab1_1 = PhotoImage(file=[int(config_read('section0','1')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab1_1 = Label(tab1, image=img_tab1_1)
pic_tab1_1.grid(column=1,row=2)
# 2nd combobox with picture 
box_2 = ttk.Combobox(tab1, value=list_combo, state='readonly')
box_2.set(list_combo[int(config_read('section0','2'))])
box_2.bind('<<ComboboxSelected>>', lambda evente: config_write('section0','2',box_2.current()))
box_2.grid(column=2, row=1)
img_tab1_2 = PhotoImage(file=[int(config_read('section0','2')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab1_2 = Label(tab1, image=img_tab1_2)
pic_tab1_2.grid(column=2,row=2)
# 3rd combobox with picture 
box_3 = ttk.Combobox(tab1, value=list_combo, state='readonly')
box_3.set(list_combo[int(config_read('section0','3'))])
box_3.bind('<<ComboboxSelected>>', lambda evente: config_write('section0','3',box_3.current()))
box_3.grid(column=3, row=1)
img_tab1_3 = PhotoImage(file=[int(config_read('section0','3')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab1_3 = Label(tab1, image=img_tab1_3)
pic_tab1_3.grid(column=3,row=2)
# 4th combobox with picture 
box_4 = ttk.Combobox(tab1, value=list_combo, state='readonly')
box_4.set(list_combo[int(config_read('section0','4'))])
box_4.bind('<<ComboboxSelected>>', lambda evente: config_write('section0','4',box_4.current()))
box_4.grid(column=4, row=1)
img_tab1_4 = PhotoImage(file=[int(config_read('section0','4')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab1_4 = Label(tab1, image=img_tab1_4)
pic_tab1_4.grid(column=4,row=2)

#---Tab2------------------------------------------------------------------

img_tab2_1 = PhotoImage(file=[int(config_read('section0','1')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab2_1 = Label(tab2, image=img_tab2_1)
pic_tab2_1.grid(column=1,row=1)

img_tab2_2 = PhotoImage(file=[int(config_read('section0','2')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab2_2 = Label(tab2, image=img_tab2_2)
pic_tab2_2.grid(column=2,row=1)

img_tab2_3 = PhotoImage(file=[int(config_read('section0','3')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab2_3 = Label(tab2, image=img_tab2_3)
pic_tab2_3.grid(column=3,row=1)

img_tab2_4 = PhotoImage(file=[int(config_read('section0','4')),'_pic.png']) # 1 _pic.png,2 _pic.png,3 _pic.png,4 _pic.png
pic_tab2_4 = Label(tab2, image=img_tab2_4)
pic_tab2_4.grid(column=4,row=1)

mainloop()

[Sirius3]

@majorkeen: außer Definitionen und Konstanten sollte nichts auf oberster Ebene stehen. Zeilen 8-22 und alles ab Zeile 34 gehört also in eine Funktion, die man üblicherweise »main« nennt und am Schluß der Datei aufruft. Eingerückt wird grundsätzlich mit 4 Leerzeichen pro Ebene. Wenn man anfängt Variablennamen durchzunummerieren, will man eigentlich Listen verwenden und wenn man Code mehrfach nur mit leichten Änderungen hat, eigentlich Schleifen. Zeile 40 ist überflüssig und die for-Schleife in Zeile 41 ein Anti-Pattern, da man gleich direkt über die Liste iterieren kann. Braucht man zusätzlich einen Index, nimmt man »enumerate«. Variablen betreten eine Funktion über ihre Argumente, »config« in »config_write« und »config_read« taucht einfach so auf, was man auch sofort merkt, sobald Zeile 19 in eine Funktion gewandert ist. »config_read« ist eigentlich überflüssig, da man die Konfigurationsdatei nur einmal laden sollte und dann nur noch »config.get« in dieser Funktion überig bleibt. Datentypen in Variablennamen zu stecken, ist unschön, da sich der Typ auch leicht mal ändern kann. Listen nennt man üblicherweise nach ihrem Inhalt im Plural: list_col -> columns. list_combo -> options. Die Kommentar am Ende der Zeilen sind alle uninformativ. Die Kommentar, die als Trenner dienen zeigen eigentlich, dass der Code zu lange ist und in mehrere Funktionen gegliedert werden sollte. Dann dient der Funktionsname der Funktion, die jetzt ein Kommentar erfüllen muß. Die lambda-Funktion in combo-select ist eigentlich schon zu komplex. Um das übersichtlicher zu gestalten braucht man eigentlich Klassen, ohne die man bei GUI-Programmierung nicht weit kommt.

Code: Alles auswählen

from tkinter import ttk
import tkinter as tk
import configparser
from functools import partial

def config_write(config, section, name, parameter):
    print('write config to:', section, name, parameter)
    config.set(section, name, parameter)
    with open('configuration.cfg', 'w') as configfile:
        config.write(configfile)

def select_image(config, boxes, images, option, index, _event):
    box, pic1, pic2 = boxes[index]
    image = images[box.current()
    config_write(config, 'section0', option, image)
    pic1['image'] = pic2['image'] = images[image]

def main():
    options = ["option1", "option2", "option3", "option4"]

    root = tk.Tk()
    root.title('ComboTest')

    images = [
        tk.PhotoImage(file='{}_pic.jpg'.format(img_index))
        for img_index in range(1,5)
    ]

    tabs = ttk.Notebook(root)
    tab1 = ttk.Frame(tab)
    tab2 = ttk.Frame(tab)

    tabs.add(tab1, text='Tab1')
    tabs.add(tab2, text='Tab2')
    tabs.grid(row=0, column=0)

    config = configparser.RawConfigParser()
    config.read('configuration.cfg')

    tk.Label(tab1, text='xxx').grid(column=0, row=0)
    tk.Label(tab1, text='row1').grid(column=0, row=1)
    tk.Label(tab1, text='row2').grid(column=0, row=2)

    boxes = []
    for idx, option in enumerate(config.options('section0'), 1):
        img_index = config.getint('section0', option)
        tk.Label(tab1, text='col{}'.format(option)).grid(column=idx, row=0)
        box = ttk.Combobox(tab1, value=options, state='readonly')
        box.set(options[img_index])
        box.bind('<<ComboboxSelected>>', partial(config, boxes, option, img_index))
        box.grid(column=idx, row=1)
        pic1 = tk.Label(tab1, image=images[img_index])
        pic1.grid(column=idx, row=2)
        pic2 = tk.Label(tab2, image=images[img_index])
        pic2.grid(column=idx, row=1)
        boxes.append(box, pic1, pic2)

    root.mainloop()

if __name__ == '__main__':
    main()

[BlackJack]

@majorkeen: Auf Modulebene gehört nur Code der Konstanten, Funktionen, und Klassen definiert. Wenn man da Variablen, Code und Funktionsdefinitionen munter mischt, steigt man ziemlich schnell nicht mehr durch wie das alles zusammenhängt. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst. Also so ähnlich wie bei C nur das es in Python nur eine Konvention ist.

Funktionen (und Methoden) sollten alles was sie verwenden (ausser Konstanten) als Argumente übergeben bekommen. Sonst hat man undurchsichtige Zusammenhänge zwischen Funktionen und kann die schlechter testen und/oder wiederverwenden. Die beiden Funktionen sollten die Konfiguration also übergeben bekommen.

`tkinter` wird üblicherweise als `tk` importiert und der Inhalt des Moduls dann darüber angesprochen.

Zwischen Funktionen und öffnende Klammer vom Aufruf schreibt man normalerweise kein Leerzeichen.

Bei dem ``return`` sind unnötige Klammern um den Rückgabewert.

Das mit der Konfiguration ist fehleranfällig. Wenn man nicht daran denkt mindestens einmal irgendeinen Wert abzufragen bevor man einen setzt, dann wird eine vorhandene Konfiguration überschrieben und es können Werte verloren gehen. Ist das überhaupt nötig, dass die Konfiguration bei jedem Abfragen eines Wertes neu gelesen wird?

Daten und Code sollte man nicht wiederholen. Das ist und macht unnötige Arbeit und ist eine Fehlerquelle beim Entwickeln und Verändern des Programms. Man kann Daten als Konstanten heraus ziehen und Codewiederholungen durch Schleifen und/oder Funktionen vermeiden.

Das was Du `tab` nennst ist kein Reiter sondern der ”Behälter” für die Seiten/Reiter.

Durchnummerieren von Namen ist keine gute Idee. Da sollte man sich entweder bessere Namen ausdenken, oder man will dort eigentlich eine Datenstruktur verwenden, statt einzelner Namen. Meistens eine Liste.

Man könnte sich die Kommentare mit Tab1 und Tab2 sparen wenn man die `Frame`\s dafür nicht schon so weit am Anfang definiert, sondern erst dann, wenn sie auch tatsächlich gebraucht werden. `list_combo` und `list_col` sind auch ein bisschen früh definiert. Zudem sollte man konkrete Grunddatentypen nicht in den Namen schreiben. Namen sollen dem Leser die Bedeutung des Wertes vermitteln.

``x = 0`` for der ``for``-Schleife mit `x` als Laufvariable macht hier keinen Sinn. Das würde nur Sinn machen wenn man `x` nach der Schleife noch braucht *und* es passieren kann, dass das iterierbare Objekt nach ``in`` leer ist, der Name also durch die Schleife nicht gebunden werden würde.

Solche Zählschleifen um mit der Laufvariablen als Index auf Sequenzen zuzugreifen sind in Python *sehr* selten, denn man kann *direkt* über die Elemente iterieren. Wenn man *zusätzlich* eine laufende ganze Zahl benötigt, macht man das mit `enumerate()`.

Bei den Comboboxen gibt es viel sich wiederholenden Code den man mit einer Schleife vermeiden kann, wobei das aber anfängt ein wenig eklig zu werden was das binden der Werte bei den ``lambda``-Ausdrücken angeht. Das ist der Punkt an dem man eigentlich auf objektorientierte Programmierung (OOP) setzt, weil das anfängt sonst ein bisschen unhandlich zu werden. Bei GUI-Programmierung kommt man da nicht wirklich drum herum.

Was Du da mit Listen als Argumente für Dateinamen machst ist ziemlich undurchsichtig weil es sich auf das Verhalten von Tk/Tcl verlässt, was nicht so viele Python-Programmierer verstehen dürften. Da sollte man die Dateinamen als Zeichenkette in Python erstellen.

Statt INI-Dateien würde ich JSON als Format verwenden. Das ist flexibler und kennt zudem Datentypen wie Zahlen.

Ungetestet:

Code: Alles auswählen

import configparser
import tkinter as tk
from tkinter import ttk

CONFIG_FILENAME = 'configuration.cfg'


def config_write(config, section, name, parameter):
    print('write config to:', section, name, parameter)
    config.read(CONFIG_FILENAME)
    config.set(section, name, parameter)
    with open(CONFIG_FILENAME, 'w') as configfile:
        config.write(configfile)


def config_read(config, section, name):
    print ('read config from:', section, name)
    config.read(CONFIG_FILENAME)
    return config.get(section, name)


def main():
    root = tk.Tk()
    root.title('ComboTest')

    config = configparser.RawConfigParser()
    images = [tk.PhotoImage(file='{}_pic.png'.format(i)) for i in range(1, 5)]

    notebook = ttk.Notebook(root)
    notebook.grid(row=0, column=0)

    tab1 = ttk.Frame(notebook)
    notebook.add(tab1, text='Tab1')

    tk.Label(tab1, text='row1').grid(column=0, row=1)
    tk.Label(tab1, text='row2').grid(column=0, row=2)

    for i, column_name in enumerate(['row1', 'col1', 'col2', 'col3', 'col4']):
        label = tk.Label(tab1, text=column_name)
        label.grid(column=i, row=0)

    options = ['option1', 'option2', 'option3', 'option4']
    for i in range(1, 5):
        name = str(i)
        option_index = int(config_read(config, 'section0', name))
        box = ttk.Combobox(tab1, value=options, state='readonly')
        box.set(options[option_index])
        box.bind(
            '<<ComboboxSelected>>',
            lambda _event, name=name, box=box:
                config_write(config, 'section0', name, box.current())
        )
        box.grid(column=i, row=1)
        tk.Label(tab1, image=images[option_index]).grid(column=i, row=2)

    tab2 = ttk.Frame(notebook)
    notebook.add(tab2, text='Tab2')

    for i in xrange(1, 5):
        image = images[int(config_read(config, 'section0', str(i)))]
        tk.Label(tab2, image=image).grid(column=i, row=1)

    root.mainloop()


if __name__ == '__main__':
    main()

Zum auswechseln der angezeigten Bilder müsste man sich die `Label`-Exemplare merken und die 'image'-Option ändern. Entweder über Schlüsselzugriff (``label['image'] = neues_bild``) oder mit der `config()`-Methode (``label.config(image=neues_bild)``).

[majorkeen]

@Sirius3 @BlackJack:
Vielen Dank für die sehr ausführlichen Beispiele, Beschreibungen und Konstruktiven Hinweise.
Die werde ich nun verstehen und anwenden.
Gruß Holgus

[majorkeen]

@Sirius: bei deinem Code hänge ich beim partial in der Zeile 50 mit folgender Fehlermeldung: TypeError: the first argument must be callable. Erwartest du hier die Ausführung von select_image?
@BlackJack: bei deinem Code würden die Images in Tab1 und auch Tab2 nicht aktualisiert, nur beim Neustart des Programms. Wie würde man dies realisieren wenn beim Combobox Event die Images ausgetauscht werden sollen. 2. Frage aus meinem 1. Beitrag

Vielen Dank ! Gruß Holgus

[Sirius3]

@majorkeen: genau »partial(select_image, config, boxes, option, img_index)«.

[majorkeen]

Mein Code ist dank eurer Hilfe nun da wo er sein soll und funktioniert. Vielen Dank!

@Sirius: mit dem »partial(select_image, config, boxes, option, img_index)«. bin ich nicht zurecht gekommen.
Unten folgend nach meinem Kenntnisstand die Umsetzung des Codes.

Nun habe ich trotzdem noch ein Paar fragen dazu:
1. Der unten folgende Code ist aufgrund der Verschachtelungen von Unterfunktionen in der Main, schlecht geschrieben und für mich ein Wunder das es überhaupt geht... Mir ist noch nicht klar wie ich das Variablen Problem umgehe?
2. Warum funktioniert der Teil #?1 nicht. Für mich ist "box.bind('<<ComboboxSelected>>', lambda _event,..." eine art Interrupt. Da sollte es doch eigentlich egal sein ob sich dieser Teil in einer For schleife befindet oder außerhalb. Der Teil wird noch nicht einmal angesprungen. Warum geht das nicht?
3. Wie würde man diesen Code in Klassen umstrukturieren? Wie würde eine professionelle Umsetzung aussehen?

Vielen Dank!
Gruß Holgus

Mein configuration.cfg:

Code: Alles auswählen

[section0]
1 = 2
2 = 2
3 = 4
4 = 4
5 = 1
6 = 2
7 = 3
8 = 2
9 = 1
10 = 4
11 = 1
12 = 2
13 = 1
14 = 1
15 = 1
16 = 1
17 = 1
18 = 1
19 = 1
20 = 1

Mein Python:

Code: Alles auswählen

from functools import partial
import configparser
from tkinter import *
from tkinter.ttk import *


options = [
    "---",
    "Option1",
    "Option2",
    "Option3",
    "Option4"
    ]

tab1=0
tab2=0
tab3=0
images = 0

INDEX_MAX_IMG = 19
INDEX_ROW_IMG = 6


def main():
  
    root = Tk()
    root.title('Test')

    config = configparser.RawConfigParser()
    config.read('configuration.cfg')
    
    images = [PhotoImage(file='{}_pic.png'.format(img_index))for img_index in range(0,len(options))]

    notebook = Notebook(root)
    tab1 = Frame(notebook)
    tab2 = Frame(notebook)
    tab3 = Frame(notebook)

    
    notebook.add(tab1, text='Tab1')
    notebook.add(tab2, text='Tab2')
    notebook.add(tab3, text='Tab3')

    notebook.grid(row=0, column=0)
    
    Label(tab2, text='xxx').grid(column=0, row=0)
    Label(tab2, text='image row').grid(column=0, row=1)
    Label(tab2, text='Auswahl').grid(column=0, row=2)
    Label(tab2, text='image row').grid(column=0, row=3)
    Label(tab2, text='Auswahl').grid(column=0, row=4)

    def calc_matrix_position(index_box, start_column, start_row, distance_row):
        column = index_box + start_column
        row = start_row
        if index_box > (INDEX_ROW_IMG*2):
            column = column - 12
            row = start_row + 2 + 2*distance_row 
        elif index_box > INDEX_ROW_IMG:
            column = column - 6
            row = start_row + 1 + distance_row 
        return(column,row)

    def config_write(config, section, name, parameter):
        print('write config to:', section, name, parameter)
        config.set(section, name, parameter)
        with open('configuration.cfg', 'w') as configfile:
            config.write(configfile)

    def set_picture (config, tab, index_image, section, name, parameter, start_column, start_row, distance_row):
        print('set_picture name:',name)
        if  type(section) == str and config.get(section, name) != parameter:
            config_write(config, section, name, parameter)
        column, row = calc_matrix_position(int(name), start_column, start_row, distance_row)
        Label(tab, image = index_image).grid(column=column, row=row)
        return(column,row)

    def update_tab(config,tab):
        print('updatetab',tab)
        if tab == 0:
            print('Tab2',tab)
            for i in range(1, INDEX_MAX_IMG):
                set_picture(config, tab1, images[config.getint('section0', str(i))],0,i,0,0,1,0)

    for i in range(1, INDEX_MAX_IMG):
        name = str(i)
        option_index = config.getint('section0', name)
        column, row = set_picture(config, tab2, images[option_index],0,i,0,0,1,1)
        box = Combobox(tab2, value=options, state='readonly')
        box.set(options[option_index])
        box.grid(column=column, row=row+1)
        box.bind('<<ComboboxSelected>>', lambda _event, name = name, box = box:
                 set_picture(config, tab2, images[box.current()], 'section0', name, box.current(),0,1,1))
    
    for i in range(1, INDEX_MAX_IMG):
        set_picture(config, tab1, images[config.getint('section0', str(i))],0,i,0,0,1,0)

    #?1 box.bind('<<ComboboxSelected>>', lambda _event, name = name, box = box:
    #?1     set_picture(config, tab2, images[box.current()], 'section0', name, box.current(),0,1,1))
    notebook.bind('<<NotebookTabChanged>>', lambda _event: update_tab(config,notebook.index("current")))

    root.mainloop()
    
if __name__ == '__main__':
    main()

[Sirius3]

@majorkeen: ein Forum ist der falsche Ort, um zu lernen, wie man Klassen programmiert. Dafür gibt es Tutorials und Bücher. Erst wenn Du die Grundlagen beherrschst, macht es Sinn, hieran weiterzuarbeiten.

[BlackJack]

@majorkeen: `tab1` bis `tab3` und `images` auf Modulebene machen keinen Sinn.

`calc_matrix_position()` ist eine normale, unabhängige Funktion die nicht in `main()` gehört.

Ad 2.: Was ist für Dich eine Art Interrupt und warum denkst Du es sei egal ob das in oder nach der Schleife steht?

Das wird auch nach der Schleife ausgeführt und hat dort auch einen Effekt, das heisst die ``lambda``-Funktion wird auch ausgeführt wenn man etwas aus *der* `Combobox` auswählt bei der man dieses Ereignis an die Funktion gebunden hat. Halt nicht auf magische Weise auf allen `Combobox`\en.

Wenn man einen Wert an einen Namen bindet, dann ist danach genau dieser eine Wert an den Namen gebunden. Falls der Namen vorher an einen Namen gebunden war, dann hat das keinen Einfluss auf das alte oder neue Objekt das an diesen Namen gebunden wird.