bad performance with tkinter text widget ?

[Xynon1]

1) Die neue Klasse vererbt das Text Widget, und Textwidget und die beiden Scollbars werden in einen Frame gepackt.

Nicht ganz, aber fast, also die Klasse erbt von dem Text-Widget und erstellt einen Frame über sich, also in dem Fall direkt auf root.
Auf diesen Frame, wie du richtig erkannt hast, liegen dann das Text-Widget mit den Scrollbars.

Strucktur wird also "root --> Frame --> (Text, Scroll1, Scroll2)"
Wichtig ist, nur das die Klasse selbst, von Textwidget erbt.
Aber du hast es schon richtig erkannt.

2) Warum hat die init in der Argumentliste rows, columns, und das cnf-dictionary, koennte man nicht auch die rows und columns ueber das dictionary mitgeben, oder ist das mehr easy to use fuer die rows und columns?

1. Richtig, "easy to use".
2. weil es für mich schon von Bedeutung ist, das bei zB. Terminal die Angaben bei der Definition gemacht werden
3. bei width und height, denkt man nicht immer sofort an Zeilen und Spalten sondern an Pixel und vondaher ist es etwas verständlicher.

Kannst du aber gerne heraus nehmen.

3) du benutzt hier intern den pack Manager, kann ich trotzdem den Grid Manager
von aussen fuer mich benutzen?

Hast du ja schon herrausgefunden, hierzu solltest du die Packmanager Theorie verstanden haben.
Ein Widget, kann immer nur einen Packmanager nutzen, du kannst also auf jedem Widget einen anderen Packmanager nutzen.
Aber nie zwei verschiedene auf dem selben Widget.

4) warum machst du ein __init__ auf das Text widget, koennte man da nicht auch ein __init__
auf den Frame machen und danach ein Text widget definieren, evtl: self.my_text = Text(self) ?

OOP - MyScrolledText, muss von Text erben.
Und wir wollen ein Textwidget keinen Frame, sonst könnte man das auch anders machen, das ist schon richtig.
Aber du musst dir immer ansehen was du brauchst --> ein TextWidget und kein Frame mit einem TextWidget darauf.
Letztendlich dient es nur zur Vereinfachung der Ansteuerung.

5) die beiden Zeilen:

Code: Alles auswählen

            
cnf.update({"yscrollcommand": self.vbar.set})
self.vbar.configure(command=self.yview)

sehen irgendwie doppelt gemoppelt aus, du setzt doch in der ersten schon das yscrollcommand,
warum dann noch das configure in der naechsten Zeile?

Sie mal bitte genau hin was dort passiert,
- bei configure gebe ich das Scrollcommando ein, ohne dieses könntest du die Scrollbars nicht bewegen.
- bei cnf.update setze ich das commando für das TextWidget, da cnf an dieses weiter gegeben wird und setze die Scrollbar auf den ensprechenden Wert der Scrollregion des TextWidgets.
Ohne dieses könntest du zwar in dem TextWidget scrollen, würdest aber kein Feedback bekommen, da die Scrollbars immer auf 0 stehen.
Spricht diese würden sich nicht wirklich bewegen.

6) Die method loop sieht aus, als wenn du da alle Layout Manager mit abdecken willst,
aber was genau da passiert versteh ich leider nicht, setzt du da die Location des Frames?
Leider laeuft das ganze auch nicht, er bringt mir hierbei:

Code: Alles auswählen

    
methods = vars(Pack).keys() + vars(Grid).keys() + vars(Place).keys()
NameError: global name 'Pack' is not defined

Ja, richtig hier ist mir ein kleiner Fehler unterlaufen, weil ich die Packmanager noch im Bytecompile hatte.
Dort muss es heißen tkinter.Pack, tkinter.Grid, tkinter.Place <-- werde das gleich nochmal im Quelltext oben ändern.

Was mach es --> es mach nichts weiter als die Packmanager vom Frame auch für die TextWidget geltend zu machen.

7)
self.text = "" loeschst du damit erst mal grundsaetzlich den Inhalt? Muss das immer dort
stehen weil du ja in der Funktion create auch hast:
self.delete(1.0, END)
Oder brauch man das ganze, damit fest steht, das der Inhalt ein String ist?

Nein, damit lege ich fest, das jede Instanz dieser Klasse auch wirklich ein "text"-Attribute hat.
Ist bei OOP üblich diese mit leeren Werten zu initialisieren, auch wenn man es in Python jederzeit ergänzen könnte, dies hat aber vorallem den Vorteil Fehler zuvermeiden.
Das self.delete leert nur das Fenster, du musst hier wirklich beachten, das das Attribute "text" nichtsmit dem Text in dem TextWidget zutun hat, sondern lediglich als speicher dient, um den eigentlichen Text anzuzeigen.
Du kannst das Text Attribute ignorieren oder löschen, aber ich finde es einfacher zu sagen

Code: Alles auswählen

textwidget.text = "abc"
textwidget.create()

als jedes mal zu schreiben

Code: Alles auswählen

textwidget.config(state=NORMAL)
textwidget.delete(1.0, END)
textwidget.insert(END, "abc")
textwidget.config(state=DISABLED)

Du kannst dir die Methode auch umstricken, das firt last war ja nur auf dein urspüngliches Problem, das die Performance nicht ausreichte.
Du könntest es zB. so machen das du nur noch create("abc") machen musst.
Ist dir überlassen.

Wenn du in den Kommentaren schreibst: Text anzeigen
Ist damit gemeint, den gesamten Text laden oder nur den Ausschnitt?

Meinte das TextWidget damit, also welcher Text wirklich angezeigt werden soll.
siehe 7. dazu nochmal.

9)
in der main hast du: text_options, die setzen width und height auf 110 und 50,
dann setzt du 50 und 80 fuer rows und columns, das versteh ich nicht, welches ist
davon das wirklich benutzte? Und wie passen beide zusammen?

Richtig, danke noch so ein kleiner Fehler von mir,
wenn du hinsiehst, werden die Einträge in width und height überschrieben.
werde dies auch gleich richtig stellen.

Danke für den Hinweis.

[ts7343]

Hallo,

ich hab den Code noch etwas umgestrickt, speziell die Methoden fuer die Anzeige,
hoffentlich schlaegst du nicht kopfschuettelnd beide Haende ueber den Kopf, was ich denn mit
deinem orig code gemacht habe, deshalb lieber noch ein paar Erlaeuterungen:

1) die Funktionsbeschreibungen sind bei mir anders aufgesetzt, deshalb hab ich deine
rausnehmen muessen, am Ende werden sie so ungefaehr aussehen:

Code: Alles auswählen

   def check_lsf_7(self):
      """check_lsf_7(self):
      ABSTRACT: check lsf 7
      ARGUMENTS: 
      RETURN:    
      REVISION  DATE           AUTHOR
      1         
      REVISION number and reason
      1    creation"""

2)
das eigentliche Problem was ich noch habe ist, wenn ich ueber eine Liste das ganze
darstellen will. Wenn ich deine orig Routine nehme, bekomme ich halt das ganze im Textwidget
angezeigt als: [' any string ',' next any string ']
desweiteren wusste ich nicht, wie ich das coloring (also die tags) einbinden sollte, wenn ich
das ganze schon als komplett string "" habe,
also die Funktion: text_from_list
was sagst du dazu als Profi, dolle umstaendlich? Hab ich deine Klasse damit verhunzt oder geht
es gar nicht anders und ich hab es vernuenftig realisiert?
So richtig gluecklich bin ich mit der Loesung aber nicht, da diese Zeile:
self.text = text_list
ja eigentlich nicht typenkonform und damit unsauber ist, da ich ja self.text nun eine Liste zuweise,
obwohl wir ja anfangs in der init sagen:
self.text = ""
es funktioniert zwar intern, aber eine strengere Programmiersprache wuerde mir das komplett um die
Ohren hauen, weil ich die Typendefinition nicht einhalte, wenn du da noch einen guten Hinweis haettest?

Code: Alles auswählen

#!/usr/bin/env python

import string
from Tkconstants import *
import Tkinter as tkinter

class XScrolledText(tkinter.Text):

   def __init__(self, master, rows, columns, cnf=()):
      self.frame = tkinter.Frame(master)     
      self.vbar  = tkinter.Scrollbar(self.frame)
      self.hbar  = tkinter.Scrollbar(self.frame, orient=HORIZONTAL)     
      self.vbar.pack(side=RIGHT, fill=Y)
      self.hbar.pack(side=BOTTOM, fill=X)
     
      cnf["width"] = columns
      cnf["height"] = rows
      cnf.update({"yscrollcommand": self.vbar.set})
      cnf.update({"xscrollcommand": self.hbar.set})
         
      tkinter.Text.__init__(self, self.frame, cnf)

      self.pack(side=LEFT, fill=BOTH, expand=YES)     
      self.vbar.configure(command=self.yview)
      self.hbar.configure(command=self.xview)
     
      methods = vars(tkinter.Pack).keys() + vars(tkinter.Grid).keys() + vars(tkinter.Place).keys()
      for m in methods:
         if m[0] != '_' and m != 'config' and m != 'configure':
             setattr(self, m, getattr(self.frame, m))
               
      self.text = ""


   def text_from_file(self, textfile):
      with open(textfile) as view_file:
         self.text = view_file.read()
      self.config(state=NORMAL)
      self.delete(1.0, END)     
      self.insert(END, self.text)
      self.config(state=DISABLED)


   def text_from_list(self, text_list, keyword, color):
#     predefinition
      self.text = text_list
      counter       = int(0)
      first_finding = ""
      if (color == ""):
         color = "FFEE00"
#     general fill of text widget                  
      self.config(state='normal')
      self.delete(0.0,"end")
      for i,line in enumerate(self.text):
         self.insert('end', line)
         self.see('end')
      self.config(state='disabled')
#     if there is a keyword, coloring is requested           
      if (len(keyword)>0):
         counter = int(0)
         for line in self.text:
            counter = counter + 1
            pos = string.find(string.upper(line),string.upper(keyword))
            if (pos >= 0):
               if (first_finding == ""):
                  first_finding = str(counter) + "." + str(pos)                  
               self.tag_add("highlight",str(counter) + "." + str(pos), \
                                        str(counter) + "." + str(pos + len(keyword)))
               self.tag_config("highlight", foreground=color)
               self.tag_config("highlight", font=("courier",8,"bold"))
               self.see(first_finding)

[Xynon1]

Ok, ein paar Punkte hätte ich.
0. Wie die Beschreibung aussieht, ist völlig egal, hauptsache sie ist verständlich.

1. text_from_file - würde ich nicht mit create zusammen schmeißen, da du gesagt hattest, das der text nicht immer aus einer Textdatei kommt.
Bei der Liste kann man ja dennoch eine eigene Funktion schreiben, aber du könntest ja auch mal direkten Text oder einen anderen String haben, vondaher würde ich das auslesen des Textes und schreiben in das TextWidget hier trotz allem getrennt halten.

2.
Du hast doch schon wieder das unnötige enumerate drin.

Code: Alles auswählen

      for i,line in enumerate(self.text):
         self.insert('end', line)
         self.see('end')

Warum ?

3. Auch wenn es nur Kommentare(#) sind, dennoch einrücken.

4. Ok, zum Farbversuch
Ich faß mal kurz zusammen, korrigiere mich falls ich falsch liege.

4.1 Du gibst eine Liste, welche aus Textfrakmenten besteht an die Funktion
4.2 Wenn keine Farbe gesetzt ist, setzt du eine (schon mal über default parameter nachgedacht ?)
4.2 Dann fügst du den gesamten Text dennoch in das TextWidget, und schaltest damit das Puffern dieser wieder aus, wo durch du eine miese Performance bekommst.
4.3 Dann prüfst du dein keyword auf eine bestimmte Länge
4.4 Dann eine counter anlegen, ok, aber wieso den Integer 0 in einen integer umwandeln
4.5 Jetzt gehst du wieder den Text durch, und erhöhst den counter, hier wäre doch nun ein enumerate nützlich ?!
4.6 Jetzt kommen nur noch verschieden komplizierte Positionierungen, die ja leider von dem TextWidget so vorgegeben werden.
4.7 Warum färbst du den selben Tag immer wieder ?
Wenn du mit tag_add agierst, wird eine numeration in der textbox hinterlegt, tag1, tag2, tag3, ..., diese haben jeweils den tag "highlight", also wenn du das ding färben willst musst du nur alle tags "highlight" färben, und alle tags, mit dem Wert "highlight" verhalten sich gleich.
Für eine zweite Farbe brauchst du einen zweiten tag.

Generell, finde ich das nun nicht gerade die beste Idee, einige Kritikpunkte habe ich schon oben genannt.
Also an deiner Stelle würde ich mir gut überlegen was du dort wirklich brauchst.

Zum einen hast du Liste mit Text und diese willst du in das TextWidget bringen,
dafür solltest du dir die ".join(...)"-Methode für Strings ansehen.
Damit kannst du eine Liste relativ schnell in einen String umwandeln.
zB. self.text = " ".join(text_list)

So, dann willst du bestimmte Keywords markieren, also im Prinzip eine eigenständige Methode.
Dazu brauchst du die Positionen der Tags, welche du über add_tag erstellen musst, richtig ?

Also du hast ein Wort, dieses Wort suchst du im gesamten Text, dafür dient ja deine "pos = string.find(string.upper(line), string.upper(keyword))"
Ich würde das mit Regularexpressions aus dem Modul "re" arbeiten, leider müssen die Tags immer in einem ziemlich "bekloppten" Index angeben werden.
Zeile.Spalte, wie du ja schon richtig bemerkt hast.
Zu dem hat das TextWidget selbst eine ".search"-Methode, welche aber auch nur ein Index findet und zudem noch warten muss bis der Text vollständig erzeugt wurde.

So bleibt als einzig performante Option nur "re" mit "finditer()", allerdings sieht das etwas umständlich aus dann einen dieser "bekloppten" Indexe zu bilden.
zB so:

Code: Alles auswählen

   def highlight(self, keyword, color):
      lines_length = []
      for line in self.text.split("\n"):
         lines_length.append(len(line)+1)
      
      line = 1
      for match in re.finditer(keyword, self.text):
         start = match.start()
         while start - sum(lines_length[:line]) >= 0:
            line += 1

         start -= sum(lines_length[:line-1])
         start_index = "{0}.{1}".format(line, start)
         end_index = "{0}.{1}".format(line, start + len(keyword))
         self.tag_add(keyword, start_index, end_index)
            
      self.tag_config(keyword, foreground=color)
      self.tag_config(keyword, font=("courier",8,"bold"))

Allerdings wird das bei ca. 10 MB Text dennoch nicht sonderlich schnell sein, bei 2 MB geht es flüssig.
Hat aber den Vorteil, das du es mehrmals, und mit mehreren Wörtern machen kannst.
Aber hier würde ich dir dringend raten den Text vieleicht doch zu spliten oder für das highlighting eine threads zunutzen. Letzteres, weiß ich leider nicht wie sich das auf das TextWidget ausübt.

Falls noch jemanden etwas besseres einfällt, nur zu

5. strengere Programmiersprache - was meinst du damit ?

[ts7343]

Hallo,

1.
ja hab ich deshalb auch komplett getrennt, und create nur in text_from_file uebernommen

2.
oops, das ist ein versehen,

4.
Grundsaetzlich muss ich sagen, dass nur kleinere Datenmengen gefaerbt werden sollen,
ein Beispiel waere die Anzeige eines Hilfefiiles im Textwidget, wo man davor optional
ein Keyword angeben kann. Anschliessend wird der Hilfetext entweder ohne Farbe
dargestellt oder alle Suchwoerter eingefaerbt, damit man diese im Hilfetext schneller findet.

4.2.
Parameter waere dann der naechste Schritt.
Ja das Puffern hab ich ausgestellt, da ich ja sonst davor den Text in einen String uebergeben
wuerde, ich hatte es nicht mit .join gemacht sondern ueber einen loop und dann im loop:
text = text + line (wahrscheinlich wieder langsamer als .join selbst)

4.3.
Das keyword waere quasi der evtl gesuchte Hilfetext

4.4.
Diese int(0) Umwandlung hab ich mal irgendwann machen muessen, als ich unter HPUX gearbeitet
hatte, ansonsten hat er es als string interpretiert, dann hab ich es mit der Brechstange gemacht,
und seitdem hat sich dies eingebuergert ...

4.5.
ja, das mit dem enumerate waer hier wirklich besser, das werd ich noch aendern

4.7.
das mit der numeration ueber mehrere tags kenn ich nicht, von daher hab ich es immer wieder gemacht.
das probier ich mal.


Ich find das gut, die Idee mit dem .join und dann das highlight separieren, das ist sauberer,
obwohl ich ja dann schlussendlich aus der Liste einen string mache und im highlighting wieder
eine liste erzeuge,

mit regular expr. hab ich leider noch nicht so oft gearbeitet, hat mich schon in Perl genervt,
aber bei deiner Routine sieht es gar nicht so schlimm aus,

5.
strengere Programmiersprache, damit mein ich eigentlich, dass man bei anderen Sprachen nicht
einfach von string auf liste umspringen kann, man koennte auch sagen von Zeichenkette auf Array,


Vielen Dank erst mal fuer die guten Ideen!

[Xynon1]

1. Wie schon gesagt, besser getrennt, da sie wirklich eine zutrennede Funktion bereitstellen.

4. War dem Quelltext zu entnehmen, da du nur ein Keyword einließt und keine Keywords.

4.2 Finger weg von + bei größeren Stringoperation das frisst dir die Haare vom Kopf.

4.4 Was soll ich dazu sagen:

4.7 Um das mal kurz näher zu Beleuchten, musst du wissen was ein tag ist.
Dies ist ja nichts weiter als ein Marker, wie bei CSS.
Du hast also zB <tag>Elem1</tag> <tag>Elem2</tag>, wobei in der Textbox diese Tags nur virtuell sind.
Aber das Färben tust du alle tags die <tag> heißen, intern wird es aber als tag1, tag2 gehandelt.

Ist ja auch keine wirkliche Regular Expression sondern eine sehr spezielle, diese hat aber den Vorteil, das man den Text nur einmal durchgeht, und nicht immer wieder bis zu nächsten Position und dann wieder von vorne bis zur nächsten Position.

5. Das nennt sich nicht strengere Progrmiersprache, sondern statische Typisierung,
Python ist auch streng