Label verliert image nach in root.after() Loop mit config

[Percy C]

Hallo,
Ich bin gerade dabei eine art Anzeigetafel zu programieren, welche unter anderem auch die Temperetur anzeigen soll. Für die Zahlen verwende ich Labels mit Bildern(1-9 für einer und 1-4 für zehner), da ich den Hintergrund behalten möchte und Labels immer eine Hintergrundfarbe haben. Nun habe ich einen Loop mit root.after(1000, my_loop) erstellt, damit die Labels dauernd geupdated werden, falls sich eine Zahl ändert. Dafür habe ich in den Loop .config() verwendet, damit das Labal immer das aktuelle image erhält, allerdings wenn mein Programm in den Loop geht, zeigen die Label nichts mehr an, sondern nur ihre standart Hintergrundfarbe.
Ich bin noch recht neu mir GUI´s, Ich weiß, dass mein code auch nicht der beste weg ist, eine Anzeigetafel zu programieren, weshalb ich mir wünsche, nur Hilfen für mein Problem und nicht für eventuell ineffiziente Programierung des restlichen Codes (Es sei denn ihr könnt es nicht mit ansehen ).

Code: Alles auswählen

from tkinter import *
from tkinter import ttk
from PIL import ImageTk, Image
class GUI():
    def  __init__(self):
        global new_widtha, new_heighta, new_widthb, new_heightb, temperatureeiner, temperaturezehner
        root = Tk()
        root.title("Title")
        root.geometry('600x600')
        root.attributes('-fullscreen', True)
        root.bind('<Escape>',lambda e: root.destroy())
        a = "0"
        b = "0"

        def temp():
            a = "1"
            b = "6"
            print(a+b)

        button = Button(root, command = temp)
        button.pack()

#Der Part ist für den Hintergrund, hab ich nicht selbst gemacht
        def motion(event):
            global x, y
            x, y = event.x, event.y

        def resize_image(event):
            new_width = event.width
            new_height = event.height
            background = copy_of_background.resize((new_width, new_height))
            print(new_height, new_width)
            photo = ImageTk.PhotoImage(background)
            label.config(image = photo)
            label.image = photo

        def resize_numbers(width, height):
            new_widthc = int(width * 0.96)
            new_heightc = int(height * 0.72)
            return new_widthc, new_heightc



        background = Image.open('/Solarmonitor/Textures/solarmonitor.png')
        copy_of_background = background.copy()
        photo = ImageTk.PhotoImage(background)
        label = ttk.Label(root, image = photo)
        label.bind('<Configure>', resize_image)
        label.bind("<Motion>", motion)
        label.pack(fill=BOTH, expand = YES)

#------------------------------------------------------------
        temperatur_zehner = Image.open("/Solarmonitor/Textures/zehner"+a+".png")
        temperatur_einer = Image.open("/Solarmonitor/Textures/einer"+b+".png")
        with Image.open("/Solarmonitor/Textures/zehner" + a + ".png") as img:
            widtha, heighta = img.size
            print (widtha, heighta)
        with Image.open("/Solarmonitor/Textures/einer" + b + ".png") as img:
            widthb, heightb = img.size
            print(widthb, heightb)
        new_widtha, new_heighta = resize_numbers(widtha, heighta)
        temperatur_zehner = temperatur_zehner.resize((new_widtha, new_heighta))
        temperatur_zehner = ImageTk.PhotoImage(temperatur_zehner)
        temperaturezehner = Label(root, image = temperatur_zehner, borderwidth = 0)
        temperaturezehner.place(x = 154, y = 590)
        new_widthb, new_heightb = resize_numbers(widthb, heightb)
        temperatur_einer = temperatur_einer.resize((new_widthb, new_heightb))
        temperatur_einer = ImageTk.PhotoImage(temperatur_einer)
        temperatureeiner = Label(root, image = temperatur_einer, borderwidth = 0)
        temperatureeiner.place(x = 250, y = 590)
        print("hello", temperatureeiner, temperaturezehner)
        
#Der betreffende Loop
        def my_loop():
            global temperatureeiner, temperaturezehner
            print (temperatureeiner, temperaturezehner, a, b)
            temperatur_zehner = Image.open("/Solarmonitor/Textures/zehner" + a + ".png")
            temperatur_einer = Image.open("/Solarmonitor/Textures/einer" + b + ".png")
            temperatur_zehner = temperatur_zehner.resize((new_widtha, new_heighta))
            temperatur_zehner = ImageTk.PhotoImage(temperatur_zehner)
            temperaturezehner.config(image=temperatur_zehner)
            temperatur_einer = temperatur_einer.resize((new_widthb, new_heightb))
            temperatur_einer = ImageTk.PhotoImage(temperatur_einer)
            temperatureeiner.config(image=temperatur_einer)
            print("hi", new_widtha, new_heighta, new_widthb, new_heightb)
            print(temperatureeiner, temperaturezehner)
            root.after(1000, my_loop)

        root.after(1000, my_loop)


        root.mainloop()

P.S. Die prints waren für mich als hilfe, um zu sehen, welche Variablen in den Loop übernommen wurden (alle)

[Sirius3]

Zum Code: keine Sternchen-Importe, keine verschachtelten Funktionen. Klassen sind ja gerade dazu da, dass man mehrere Methoden zusammenfasst, und dadurch die ganzen `global` vermeidet. Du hast aber eine große __init__-Funktion, die quasi einen globalen Namensraum aufspannt. Sowohl Funktion als auch Klasse könnte man sich sparen, und alles zwei Ebenen nach außen rücken, ohne etwas zu verlieren. Schreibe richtige Klassen, mit richtigen Methoden.
`a` und `b` sind keine guten Variablennamen. Man braucht immer eine Referenz auf das Image-Objekt, sonst räumt der Garbage-Collector von Python das Bild ab und die GUI kann nichts mehr anzeigen. Hier wäre es sowieso am besten, alle Bilder vorzuladen, zwischenzuspeichern und nur auszutauschen.

[Percy C]

Die Variablen a und b wollte ich sowieso noch abändern, die Classe hatte ich gemacht, damit ich sie später über ein main script ausführen kann, from tkinter import * habe ich wegen dem part den ich nicht gemacht habe gemacht, da dort auch import * verwendet wurde. Meinst du mit dem letzten Satz, dass ich für jedes bild ein objekt erstellen soll, was dann je nach wert aufgerufen wird? Falls ja wollte ich dass mit den Variablen a und b ja vereinfachen...

[__blackjack__]

@Percy C: Du hast da keine Klasse. Ja da steht das Schlüsselwort ``class``, aber semantisch ist das so überhaupt gar keine Klasse. In Klassen fasst man Daten und dazugehörige Operationen zusammen. Deine ”Klasse” hat weder Datenattribute, noch Operationen. Die einzige ”Methode” die definiert ist, macht mit dem Objekt auf dem sie ausgeführt wird genau gar nichts, ist damit also keine Methode, sondern nur eine Funktion die sinnloserweise in eine Klasse gesteckt wurde. Klassen führt man auch nicht aus. Diese Begründung für die ”Klasse” klingt also komisch. Wenn man etwas zum Ausführen haben möchte nachdem man ein Modul importiert hat, dann ist das normalerweise eine Funktion. Man kann da zum Beispiel als ersten Schritt einfach mal ``class …`` entfernen und alles eine Ebene weniger tief einrücken. Dann die `__init__()` in `main()` umbenennen und das sinnfreie, da nicht verwendete `self`-Argument entfernen. Dadurch hat sich am Ablauf des Programms nichts geändert.

Bezüglich des *-Imports: Macht man halt nicht — überdenke anderes aus der Quelle wo Du das her hast. Nicht alles was man im Netz findet ist brauchbar.

Mit dem letzten Satz war gemeint bei der Initialisierung alle Bilder zu laden und beispielsweise in einer Liste zu speichern, statt jedes mal wenn man eine andere Ziffer darstellen möchte, diese (erneut) zu laden. Das würde dann auch nebenbei das Problem lösen was der Code an der Stelle im Moment hat: Python's Speicherverwaltung weiss nicht das Tk die Bilder noch für die Anzeige benötigt. Darum muss man in Python dafür sorgen das man in Python immer noch irgendwie auf das Bildobjekt zugreifen kann solange das angezeigt werden soll.

Wenn man die ”Klasse” entfernt und deren `__init__()` zur Hauptfunktion (des Moduls) macht, hat man *eine* 100 Zeilen lange Funktion in der fünf lokale Funktionen definiert werden. Lokale Funktionen kann man schlecht testen, nicht wiederverwenden, und sie haben keine saubere Schnittstelle, denn man muss immer aufpassen ob Namen aus der umgebenden Funktion kommen oder nur lokal in der lokalen Funktion sind. Lokale Funktionen verwendet man deshalb in der Regel nur wenn die als Closure fungieren und entweder an den Aufrufer zurückgegeben oder an eine andere Funktion oder Methode als Argument weitergegeben werden. Für andere Zwecke sollte man eine gute Begründung haben warum das nicht eigenständige Funktionen sind. Bei den fünf im Beispiel sehe ich da keinen Grund, die sollten also eigenständig sein. Dann muss man natürlich auch sauber alles was die Funktionen ausser Konstanten so benötigen auch als Argument(e) übergeben.

Wobei man da auch nicht alle Funktionen braucht. `motion()` macht beispielsweise keinen Sinn. Das verunreinigt nur den Modulnamensraum mit `x` und `y` die nirgends im Programm verwendet werden. Warum?

Was soll in `resize_numbers()` eigentlich das `c` am Ende von `new_heightc` und `new_widthc` bedeuten? Bitte keine kryptischen Abkürzungen oder Pre- und Suffixe verwenden. `my` ist auch nur *sehr* selten ein sinnvoller Namenszusatz. Eigentlich nur wenn es auch `our` und/oder `their` als Zusatz gibt von dem man `my` unterscheiden möchte. Den Leser interessiert doch nicht dass das ”Deine” Funktion ist, oder dass das eine Schleife ist (was ja nicht mal wirklich stimmt) sondern was die Funktion *macht*. Also beispielsweise `update_temperature_display()` oder etwas in der Richtung.

Du hast da also `temperatureiner`, `temperaturzehner`, `temperatur_einer` und `temperatur_zehner` und ob da nun ein Unterstrich zwischen den beiden Worten ist oder nicht ist das Unterscheidungskriterium ob es sich um ein Bildobjekt oder um ein `Label` handelt‽ Wie ist denn die Entscheidung gefallen was was ist?

`a` und `b` sollten nicht überall im Programm existieren. Eigentlich gibt es die ja auch gar nicht als eigentständige Werte sondern es gibt eine Tempartur aus der diese beiden Werte berechnet werden. Und das im Grunde auch nur an einer einzigen Stelle im Programm, nämlich in `my_loop()`, weil dort die passenden Bilder zur Temperatur gebraucht werden. Du hast das vorher im Code schon mal, solltest Du aber nicht, denn das ist ja Code der sich wiederholt und Code- und Datenwiederholungen vermeidet man als Programmierer. Selbst so eine Zeile wie der `after()`-Aufruf: Statt das zweimal hinzuschreiben, einmal um die Funtkion das erste mal aufrufen zu lassen, und dann in der Funktion noch mal das gleiche damit sie wiederholt aufgerufen wird, würde man für das erste mal einfach direkt selbst die Funktion aufrufen. Dann würde dort die Temperaturanzeige auch sofort aktualisiert und man braucht den Code nicht vorher schonmal irgendwo damit nicht erst nach einer Sekunde etwas angezeigt wird.

Eine Datenwiederholung die man sich sparen sollte ist das Basisverzeichnis für die Bilder. Für "/Solarmonitor/Textures" würde man eine Konstante definieren. Wenn man das mal ändern möchte, sollte man das nur an *einer* Stelle im Programm machen müssen.

`copy_of_background` ist unnötig, da kann man auch einfach `background` nehmen.

Zwischenstand (ungetestet):

Code: Alles auswählen

#!/usr/bin/env python3
import tkinter as tk
from functools import partial
from pathlib import Path
from tkinter import ttk

from PIL import Image, ImageTk


TEXTURE_PATH = Path("/Solarmonitor/Textures")


def resize_image(label, image, event):
    photo = ImageTk.PhotoImage(image.resize((event.width, event.height)))
    label.config(image=photo)
    label.image = photo


def resize_numbers(width, height):
    return int(width * 0.96), int(height * 0.72)


def update_temperature_display(digit_labels, digits_images, temperature_var):
    for label, images, value in zip(
        digit_labels, digits_images, divmod(temperature_var.get(), 10)
    ):
        label["image"] = images[value]

    digit_labels[0].after(
        1000,
        update_temperature_display,
        digit_labels,
        digits_images,
        temperature_var,
    )


def main():
    root = tk.Tk()
    root.title("Title")
    root.geometry("600x600")
    root.attributes("-fullscreen", True)
    root.bind("<Escape>", lambda _: root.destroy())

    temperature_var = tk.IntVar(value=0)

    tk.Button(root, command=partial(temperature_var.set, 16)).pack()

    background = Image.open(TEXTURE_PATH / "solarmonitor.png")
    photo = ImageTk.PhotoImage(background)
    label = ttk.Label(root, image=photo)
    label.bind("<Configure>", partial(resize_image, label, background))
    label.pack(fill=tk.BOTH, expand=tk.YES)

    digits_images = list()
    for filename_prefix in ["zehner", "einer"]:
        images = list()
        for digit_value in range(10):
            image = Image.open(
                TEXTURE_PATH / f"{filename_prefix}{digit_value}.png"
            )
            images.append(
                image.resize(resize_numbers(image.width, image.height))
            )
        digits_images.append(images)

    temperatur_zehner_label = tk.Label(root, borderwidth=0)
    temperatur_zehner_label.place(x=154, y=590)
    temperatur_einer_label = tk.Label(root, borderwidth=0)
    temperatur_einer_label.place(x=250, y=590)

    update_temperature_display(
        [temperatur_zehner_label, temperatur_einer_label],
        digits_images,
        temperature_var,
    )
    root.mainloop()


if __name__ == "__main__":
    main()