Refactoring, oder wie macht man es schön.

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
vario
User
Beiträge: 4
Registriert: Freitag 24. Oktober 2008, 15:43

Samstag 2. Mai 2009, 12:18

Hallo an alle,

als Physiker finde ich Python sehr praktisch, wenn ich einfach loslege sieht das Ergebnis aber irgendwie nie wirklich elegant aus. Hier sind wieder einmal 30 Zeilen Code, die ich nicht besser hinbekommen habe. Das ganze konvertiert binäre Messdaten in eine leserliche ASCII-Form.

Code: Alles auswählen

# quick converter for raw-measurement files
# file format:
#
# 256 bytes header
# 56 bytes entry: 7 IEEE doubles (64 bit)
# 56 bytes entry: 7 IEEE doubles (64 bit)
# ...

import glob
from struct import unpack
from numpy import array, savetxt

def convertEntry(buffer, listArray):
  struct = '7d'
  row = unpack(struct, buffer)
  for i, item in enumerate(row):
    listArray[i].append(item)

# look for raw files
filenames = glob.glob('*.raw')

for name in filenames:
  # initialize 
  listArray = [ [], [], [], [], [], [], [] ] 
  f = open(name, 'rb')
  # seek to first entry
  f.seek(256)
  
  # read data and parse it
  structLength = 56
  while True:
    buffer = f.read(structLength)
    if buffer == '': 
      break
    convertEntry(buffer, listArray)
  
  f.close()
 
  time       = listArray[0]
  voltage    = listArray[1]
  current    = listArray[2]
  amplitude  = listArray[3]
  delay      = listArray[4]
  resistance = listArray[5]
  
  table = array([time, voltage, current, amplitude, delay, resistance]).transpose()
  
  savetxt((name +'.dat'), table, fmt='%.7e', delimiter='\t' )
Habt ihr vielleicht den ein oder anderen Vorschlag wie man das schöner erledigen kann?

Viele Grüße
vario
Benutzeravatar
b.esser-wisser
User
Beiträge: 272
Registriert: Freitag 20. Februar 2009, 14:21
Wohnort: Bundeshauptstadt B.

Samstag 2. Mai 2009, 13:09

Geht es auch so?

Code: Alles auswählen

# look for raw files
filenames = glob.glob('*.raw')
for fname in filenames:
    with open(fname, "rb") as array_file:
        array_file.seek(256)
        table = numpy.fromfile(array_file)
        # Falls du kein "name.raw.dat" willst:
        # os.path.splitext(fname)[0] + ".dat"
        numpy.savetxt(fname +'.dat', table, fmt='%.7e', delimiter='\t' )
Man muss ja nicht mehr Code schreiben als unbedingt nötig :twisted:

hth, Jörg
ps.: (in Python 2.5 brauchst du am Anfang "from __future__ import with_statement")
Wir haben schon 10% vom 21. Jahrhundert hinter uns!
Benutzeravatar
BlackVivi
User
Beiträge: 762
Registriert: Samstag 9. Dezember 2006, 14:29
Kontaktdaten:

Samstag 2. Mai 2009, 13:13

- Beachte PEP8
- Kein Code auf Modulebene... verteil die Vorgänge auf Funktionen und pack die Aufrufe in eine main Methode, die du am Ende des Moduls mit if __name__ == "__main__": main() aufrufst... schau im Forum rum, da gibts viele Beispiele dafür.
- Wenn es auf Funktionen verteilt ist, kannst du die Funktionen auch wunderbar kommentieren, jede Funktion im Kopf. Schon ist klar, was wo passiert und du kannst die Funktionen gegebenfalls woanders einsetzen.
BlackJack

Samstag 2. Mai 2009, 13:14

@vario: Vielleicht sehe ich das auch gerade falsch, aber kann es sein, dass Du die Sieben Werte einliesst, dann in sieben verschiedene Listen steckst, das in ein `numpy.array` nur um dann mit der `transpose()`-Methdode diese Verteilung auf sieben verschiedene Listen wieder *rückgängig* zu machen!?

Also ich würde das entweder ohne `numpy` machen, mit dem `csv`-Modul zum schreiben der Daten, oder ohne `struct`, weil man die Daten eigentlich auch mit `numpy` einlesen können sollte. Also Datei öffnen, an die richtige Stelle `seek()`en und dann `numpy.fromfile()`. Mit `reshape()` in die richtige Form bringen und dann `savetxt()` verwenden.

Wenn ohne `numpy`, dann würde ich das eher so schreiben, dass immer nur ein Datensatz auf einmal verarbeitet wird. Das wäre interessant, wenn die Daten wirklich gross werden und den Speicher sprengen.

Bei den Namen solltest Du auf Typen in den Namen verzichten und besser welche wählen, die den Inhalt beschreiben. Also `listArray` ist denkbar ungünstig um Rückschlüsse auf den Inhalt zu ziehen.

"Magische" Zahlen sollte man vermeiden. Die 256 und die 56 könnte man als "Konstanten" am Anfang definieren, zum Beispiel so:

Code: Alles auswählen

HEADER_SIZE = 256
ENTRY_STRUCT = struct.Struct('7d')
Die 56 versteckt sich jetzt als Attribut: ``ENTRY_STRUCT.size``. So spart man sich auch die Fehlerquelle, dass die Grösse nicht zum `unpack()` passt, wenn man sich verrechnet.

Die Lese-Schleife würde ich mit `iter()` in eine ``for``-Schleife umwandeln. Dazu braucht man dann entweder eine ``lambda``-Funktion oder `functools.partial`.

Last but not least eignen sich solche Probleme immer ganz gut für Generatoren/Iteratoren. Damit könnte das Programm zum Beispiel so aussehen (ungetestet):

Code: Alles auswählen

from __future__ import with_statement
import csv
import glob
from functools import partial
from itertools import imap
from struct import Struct

def iter_raw_file(raw_file, struct=Struct('7d'), header_size=256):
    raw_file.seek(header_size)
    return imap(struct.unpack, iter(partial(raw_file.read, struct.size), ''))

def main():
    for filename in glob.glob('*.raw'):
        with open(filename, 'rb') as raw_file:
            with open(filename + '.dat', 'wb') as dat_file:
                writer = csv.writer(dat_file, delimiter='\t')
                writer.writerows(iter_raw_file(raw_file))

if __name__ == '__main__':
    main()
vario
User
Beiträge: 4
Registriert: Freitag 24. Oktober 2008, 15:43

Samstag 2. Mai 2009, 15:36

@b.esser-wisser: Super, numpy.fromfile() habe ich übersehen, damit geht das viel einfacher. Bei deinem werden allerdings alle Zahlen in einer Spalte ausgegeben und nicht in sieben Spalten. Lässt sich das einfach beheben?

@BlackVivi: PEP8 drucke ich mir mal aus, das sollte man auf dem Schreibstisch haben. :-)

@BlackJack: Das mit den Listen und Arrays hat sich so ergeben, weil numpy.savetxt() mir das ganze in sieben Zeilen geschrieben hat. Also habe ich noch ein array dazwischen, indem mit transpose Zeilen und Spalten vertauscht werden. Der numpy.fromfile() und reshape()-Weg habe ich total übersehen, damit spart man sich vieles. Auch sowas wie ENTRY_STRUCT.size ist viel schöner als die 56, bei der habe ich mich beim ersten mal natürlich auch gleich vertan. Deine Generatoren/Iteratoren-Lösung ist elegant, komplett verstanden habe ich es noch nicht, das ist wohl eher was für den fortgeschrittenen Python-Anhänger.

Vielen Dank an alle!
Darii
User
Beiträge: 1177
Registriert: Donnerstag 29. November 2007, 17:02

Samstag 2. Mai 2009, 16:18

vario hat geschrieben:@b.esser-wisser: Super, numpy.fromfile() habe ich übersehen, damit geht das viel einfacher. Bei deinem werden allerdings alle Zahlen in einer Spalte ausgegeben und nicht in sieben Spalten. Lässt sich das einfach beheben?
Entweder per array.reshape oder indem du die Spalten explizit abgibst

Code: Alles auswählen

numpy.fromfile("file", dtype=numpy.dtype([("Spalte 1", numpy.float64), "Spalte 2", numpy.float64)])))
Benutzeravatar
b.esser-wisser
User
Beiträge: 272
Registriert: Freitag 20. Februar 2009, 14:21
Wohnort: Bundeshauptstadt B.

Samstag 2. Mai 2009, 16:54

:oops: Bei einer Binärdatei muss man die Datenstruktur natürlich angeben - da hatte ich gerade nicht dran gedacht. Darii zeigt ja wie's geht.
Wir haben schon 10% vom 21. Jahrhundert hinter uns!
Antworten