Schnellere Variante für if in;

[techma]

hallo,

ich muss aus einer txt datei zeilen auslesen. dabei steht in der ersten salte eine nummer.
die zeile interessiert mich nur, wenn die nummer der zeile auch in einer bestehenden Liste vorhanden ist.
ich habe schon etwas programmier, aber es dauert zu lange.

Code: Alles auswählen

f=open(dmpFile,'r')
lines = f.readlines()
for l in lines:
    nummer=(int(l.split()[0]))
    if nummer in Liste:
        [...]

wie kann man dies beschleunigen?

gruß!

[Hyperion]

Du könntest ein `set` benutzen - da sollte der Zugriff in O(1) klappen. Wie groß ist denn `Liste`.

Davon mal abgesehen:

- Wir haben spezielle Python-Code-Tags für Python-Quellcode

- Dateien öffnet man mit folgendem Idiom:

Code: Alles auswählen

with open(...) as handler:
    # handler ist hier file-object

- Man kann über Textdateien direkt iterieren

- Man sollte immer aussagekräftige Bezeichner verwenden `Liste` sagt eigentlich nichts aus. Man sollte auch nicht auf den Typen einer Variable im Namen eingehen... `Liste` kann sich ja auch mal ändern (so wie jetzt durch meinen Vorschlag!) - dann müsstest Du alle Vorkommnisse im Code ändern.

Das alles führt mich so zu folgendem Quellcode:

Code: Alles auswählen

with open(dmp_file, "r") as infile:
    for line in infile:
        if int(line.split()[0]) in numbers:
            pass

[deets]

@ techma

Ohne deine Ursprungsdaten zu veraendern - nichts. Du muesstest hier einen Index aufbauen, und zb in Form eines Pickles danebenlegen - dann kannst du das schneller machen. Oder die Daten gleich in eine Datenbank packen.

[Hyperion]

Ohne deine Ursprungsdaten zu veraendern - nichts.

Wenn seine `Liste` richtig groß ist, dann sollte mein Tipp schon etwas bewirken - seine Überschrift deutete ja auch dieses Problem hin. Ob das nun wirklich der Bottleneck ist, wissen wir natürlich nicht.

[deets]

@Hyperion

Oh, verzeih, das habe ich nicht richtig gelesen. Ich wuerde aber trotzdem vermuten, dass es nicht daran liegt - aber wissen kann ich das nicht.

Letztlich bleibt's dabei: mit einem Index ist die Liste von IDs das, worueber man laeuft, und holt sich die Daten in mehr oder minder konstanter Zeit.

[techma]

Danke für die Antwort, ich hab nur einen teil meines programms in abgespeckter Version gepostet hier ist nun das vollständige Programm. Es werden KnotenLabels und die Verschiebungen in x,y,z Richtung ausgelesen.
Bei der Liste handelt es sich um die keys eines Dictionaries.
Insgesamt gibt es 300 000 Knoten

Code: Alles auswählen

f=open(dmpFile,'r')
lines = f.readlines() #
zeile=0 	#werden von 0 an gezählt
dispData={}
mlist=[] 	#gibt die erste zeile für die Daten der verschiedenen Modes an
for l in lines: 
	if l.find('MODE NUMBER')>=0:
		mlist.append(zeile+5) #5 zeilen nach 'MODE NUMBER' befindet sich der erste knoten der Mode
	zeile+=1
knoten=int(lines[mlist[1]-7].split()[0]) 	#der letzte Knoten befindet sich 2 Zeilen über 'MODE NUMBER'
nodelist =[int(l.split()[0]) for l in lines[mlist[0]:mlist[0]+knoten]]
for m in mlist:
	mode=mlist.index(m)+1
	nodeData_adams={}
	nodedata1 = []
	for l in lines[m:m+knoten]:
		node=(int(l.split()[0]))
		if node in nodeData.keys():			#damit nur die SkinSetKnoten verwendet werden
			nodedata1.append(float(l.split()[1]))
			nodedata1.append(float(l.split()[2]))
			nodedata1.append(float(l.split()[3]))
			nodeData_adams[node]=nodedata1
			nodedata1 = []
	dispBib_adams[mode]=nodeData_adams	f.close()		

[Hyperion]

Wenn das doch ein Dictionary ist, wieso nutzt Du das dann nicht aus?:

Code: Alles auswählen

# so O(n) (vom Aufbauen der Liste durch `keys` mal abgesehen)
if node in nodeData.keys(): 
# so sollte es schneller und speichersparender sein:
if node in nodeData:

Zum Code könnte man noch so einiges sagen... aber da ich oben schon einiges schrieb, bin ich jetzt mal still

[jens]

Ich sehe da mehrmals sowas wie: .split()[x] Macht natürlich keinen Sinn ständig neu zu splitten

Wenn man wirklich nur das erste Element braucht, macht auch das Sinn: foo.split(None, 1)[0] Siehe auch: http://docs.python.org/library/stdtypes.html#str.split

Die Laufzeit dürfte sich dennoch nur marginal ändern.

[techma]

danke für die antworten.
mit dem Tip von Hyperion mit dem durchsuchen des dictionary geht schon sehr viel schneller. vielen dank.
Ich habe in der Datei 4 Spalten die ich benötige die erste beinhaltet die Knotennummern die 2.,3.,4. die x,y,z-Koordinaten.
und am ende brauch ich ein dictionary, welches als key die Knotennummer und als value [x,y,z] ausgibt.

Ich versuch noch die Anmerkungen von Hyperion umzusetzen, dann poste ich mal den neuen code für weitere Verbesserungsvorschläge.
'=)

[techma]

Hier das neuste Programm. den Vorschlag von Hyperion

Code: Alles auswählen

for line in infile:

konnte ich nicht einbauen, da ich dann einfach alle zeilen abarbeiten würde und nicht in allen sind von mir benötigte Daten.
gibt es noch Ideen zur Verbesserung?

Gruß!

Code: Alles auswählen

def adams_Knotenverschiebungen_auslesen(dmpFile):
	startzeit=time()
	with open(dmpFile,'r') as f:
		lines = f.readlines() #
		zeile=0 	#werden von 0 an gezählt
		dispData={}
		mlist=[] 	#gibt die erste zeile für die Daten der verschiedenen Modes an
		for l in lines: 
			if l.find('MODE NUMBER')>=0:
				mlist.append(zeile+5) #5 zeilen nach 'MODE NUMBER' befindet sich der erste knoten der Mode
			zeile+=1
		knoten=int(lines[mlist[1]-7].split()[0]) 	#der letzte Knoten befindet sich 2 Zeilen über 'MODE NUMBER'
		nodelist =[int(l.split()[0]) for l in lines[mlist[0]:mlist[0]+knoten]]
		for m in mlist:
			mode=mlist.index(m)+1
			nodeData_adams={}
			nodedata1 = []
			for l in lines[m:m+knoten]:
				node=(int(l.split()[0]))
				if node in nodeData:					#damit nur die SkinSetKnoten verwendet werden
					nodedata1.append(float(l.split()[1]))
					nodedata1.append(float(l.split()[2]))
					nodedata1.append(float(l.split()[3]))
					nodeData_adams[node]=nodedata1
					nodedata1 = []
			dispBib_adams[mode]=nodeData_adams	#32moden
		f.close()		
		endzeit=time()
		Dauer=endzeit-startzeit
		print 'Dauer für adams_Knotenverschiebungen_auslesen:',Dauer

[jens]

Ich habe in der Datei 4 Spalten die ich benötige die erste beinhaltet die Knotennummern die 2.,3.,4. die x,y,z-Koordinaten.

Dann mach doch einmal split():

Code: Alles auswählen

knoten, x, y, z = line.split()

Wie gesagt, diese ganzen .split()[0] sind unnötig.

[techma]

hallo jens,

es gibt aber noch weitere Spalten, ich benötige aber nur die 4 erwähnten.
mit dem befehl lese ich doch die komplette zeile, oder?

gruß!

[jens]

Dann geht auch sowas:

Code: Alles auswählen

knoten, x, y, z, rest = line.split(None, 4)

'rest' kannst du später mit .split() aufteilen, wenn du es brauchst.

[BlackJack]

@techma: Gibt es in der Datei neben der Teilzeichenkette 'MODE NUMBER' keine anderen Merkmale an denen man die Zeilen in Abschnitte zerlegen kann? Insbesondere das Ende eines Blocks als „zwei Zeilen vor 'MODE NUMBER'“ ist es, was es schwierig(er) macht die Datei nicht komplett einlesen zu müssen. Obwohl das auch möglich sein sollte.

Die Zeilenlänge sollte 80 Zeichen nicht überschreiten, weil der Quelltext sonst in diversen Anzeigen, zum Beispiel in Terminals, oder auch hier im Forum an Stellen umgebrochen wird, die das ganze schwieriger zu lesen machen. Wenn man die konventionellen vier Leerzeichen pro Einrückebene verwendet und die Kommentare nicht hinter die Zeilen schreibt, sondern ihnen davor eigene Zeilen spendiert, dann ist das schon einfacher einzuhalten.

Eine weitere Möglichkeit sich eine komplette Ebene zu sparen, ist es den ganzen Quelltext der nichts mit dem Einlesen der Datei zu tun hat, *hinter* den ``with``-Block zu schreiben. Das ist alles ausser der ersten und der letzten Zeile von dem Block; wobei die letzte Zeile auch noch überflüssig ist, denn das ``with`` sorgt ja gerade dafür, dass die Datei auf jeden Fall beim verlassen des Blocks geschlossen wird.

Eine weitere Formatierungsfrage sind Leerzeichen um Operatoren und Zuweisungen ausserhalb von Parameter-/Argumentlisten. Sie erhöhen die Lesbarkeit.

Statt einzelne Zeilen zu kommentieren, könntest Du auch Gruppen von Zeilen mit Erklärungen versehen warum sie das machen, was sie machen. Dann muss man sich das Gesamtbild von (Teil)Algorithmen nicht mühsam zusammen puzzlen. Zum Beispiel wäre es auch schön eine Beschreibung des Formates zu haben, bevor man anfängt den Quelltext zu lesen, der die Daten verarbeitet.

Namen sollten ausserhalb von sehr begrenzten Ausdrücken, oder wenn es einfache Zähler sind, nicht nur einen Buchstaben lang sein. `l` hat zusätzlich, dass man am Namen nicht erkennt, wofür er eigentlich steht, noch das Problem, dass man ihn in vielen Schriftarten leicht mit einer 1 verwechseln kann. Man sollte Namen nicht abkürzen, solange es sich nicht allgemein bekannte Abkürzungen handelt, und der konkrete Typ sollte auch nicht im Namen enthalten sein. Ein Negativbeispiel ist hier `mlist`. Was ist `m` und muss das immer eine Liste sein!? Wenn man nämlich später fest stellt, dass eine andere Datenstruktur effizienter ist, oder man die einfache Liste durch einen selbst implementierten Container-Datentyp ersetzt, muss man überall den Namen anpassen, wenn man keinen sehr irreführenden Namen im Quelltext haben möchte.

Wenn man aus dem `l` ein `line` macht, bekommt man das Problem, dass man in der ersten Schleife die Namen `zeile` und `line` hat, die etwas unterschiedliches bedeuten. `zeile` müsste von der Bedeutung her eigentlich `zeilennummer` heissen. Mit `knoten`/`node` wiederholt sich dieses Muster noch einmal.

Man sollte sich bei der Namensgebung mühe geben. Wenn man einen Kommentar benötigt, der erklärt wofür der Name steht, dann ist das in der Regel ein Zeichen, dass der Name besser sein könnte. Denn eigentlich sollte *der* ja schon dem Leser die Bedeutung des Wertes vermitteln, der an den Namen gebunden ist.

Eine Deutsch/Englisch-Mischung finden die meisten Leute nicht so prickelnd. Zumal das zu solchen Problemen führen kann, wie weiter oben beschrieben, dass Werte mit verschiedenen Bedeutungen den „gleichen“ Namen bekommen, oder auch umgekehrt, dass Werte mit der selben Bedeutung innerhalb eines Programms mal den englischen und mal den deutschen Namen bekommen.

Der Test ob eine Zeichenkette in einer anderen Zeichenkette enthalten ist, geht mit dem ``in``-Operator einfacher und IMHO auch lesbarer als mit der `find()`-Methode.

Das manuelle mit zählen der Zeilennummer sollte man durch die `enumerate()`-Funktion ersetzen. Dann könnte man daraus auch eine „list comprehension” (LC) machen:

Code: Alles auswählen

    mode_start_line_numbers = [
        i + 5 for i, line in enumerate(lines) if 'MODE NUMBER' in line
    ]

Die Bestimmung von `knoten` verstehe ich nicht so ganz. Es sieht so aus, als wenn es sich dabei um die Anzahl der Knoten pro „Mode” handelt. Die liesse sich doch aber auch aus den vorliegenden Zeilenangaben einfacher berechnen, und vor allem ohne dass man sich darauf verlässt welche konkreten Werte die IDs in der ersten Spalte haben!? Das hier sollte den gleichen Wert ergeben:

Code: Alles auswählen

    nodes_per_mode = mode_start_line_numbers[1] - mode_start_line_numbers[0] - 7

Weder das, noch Dein Code, ist übrigens robust gegen Dateien die weniger als zwei „Modes” enthalten.

Wenn die Node-Daten immer 5 Zeilen nach 'MODE NUMBER' anfangen und zwei Zeilen vor 'MODE NUMBER' enden, dann sollte es eigentlich auch genügen die erste, zweite und die letzte 'MODE NUMBER'-Zeile zu ermitteln. Oder die erste und die letzte und die Anzahl dieser Zeilen. Die Zeilenbereiche lassen sich daraus dann berechnen.

Was soll die Zahl im Namen von `nodedata1`? Namen durchnummerieren ist selten eine gute Idee. Der Name wird hier auch an ungünstigen Stellen an eine leere Liste gebunden. Das ist verwirrend und umständlich es einmal vor der Schleife und dann nach jedem „Gebrauch“ zu tun. So fragt man sich als Leser wo denn noch überall Werte in der Liste gesammelt werden ausser an der Stelle, wo Du die drei Koordinaten hinzufügst. Denn wenn da nicht noch andere Stellen sind, macht es keinen Sinn, dass der Code die Liste *nach* dem Gebrauch neu anlegt, und nicht *davor* und dann auch nur dort und nicht auch vor der Schleife. Letztlich ist der Name im vorliegenden Fall aber auch komplett überflüssig, denn man kann auch einfach eine anonyme, literale Liste mit den drei Werten hin schreiben.

Die Funktion enthält mit `dispData` und `nodelist` Namen, die zwar an Werte gebunden, dann aber nie wieder verwendet werden. Umgekehrt gibt es `nodeData` und `dispBib_adams` deren Herkunft ungeklärt ist. Werte die keine Konstanten sind, sollten Funktionen als Argumente betreten und nicht einfach so magisch existieren. Das macht Programme unübersichtlicher, schwerer zu verstehen, schlechter zu testen, und letztendlich fehleranfälliger wenn man etwas am Programm verändert.

Ungetestet und kommentarfrei:

Code: Alles auswählen

def adams_knotenverschiebungen_auslesen(
    node_data, disp_bib_adams, dmp_filename
):
    with open(dmp_filename) as dmp_file:
        lines = list(dmp_file)
    
    mode_start_line_numbers = [
        i + 5 for i, line in enumerate(lines) if 'MODE NUMBER' in line
    ]
    
    nodes_per_mode = mode_start_line_numbers[1] - mode_start_line_numbers[0] - 7
    
    for mode_id, mode_lines in enumerate(
        (lines[i:i + nodes_per_mode] for i in mode_start_line_numbers),
        1
    ):
        node_id2coordinates = dict()
        for line in mode_lines:
            node_id, x, y, z, _ = line.split(None, 4)
            node_id = int(node_id)
            if node_id in node_data:
                node_id2coordinates[node_id] = map(float, [x, y, z])
        disp_bib_adams[mode_id] = node_id2coordinates