@franze_m: Eingerückt wird per Konvention vier Leerzeichen pro Ebene.
`wordnr()` enthält einen ziemlich offensichtlichen Fehler, den man durch ausprobieren dieser Funktion leicht feststellen kann. Da hätte ein Unit-Test geholfen.
Auch die `read_loader()`-Funktion enthält einen offensichtlichen Fehler der eine Ausnahme zur Folge hat. Wahrscheinlich wird der momentan durch den Fehler in `wordnr()` verhindert, so dass der Code-Pfad nicht genommen wird, weil das globale `string` (was es nicht geben sollte) kein "Output_Voltage" enthält.
Letztlich ist es aber auch nicht sinnvoll Ausnahmen durch magische Fehlerwerte zu ersetzen. Insbesondere wenn der Fehlerwert auch noch an vielen Stellen als reguläres Ergebnis verwendet werden kann, ohne dass das sofort auffallen muss, und somit leicht falsche Ergebnisse entstehen können.
Der Name der Funktion ist auch nicht gut. Man sollte nicht Deutsch und English mischen. An sich ist das schon nicht gut, aber innerhalb von *einem* Namen mehrere Sprachen ist noch unübersichtlicher. Zumal das auch nicht die Nummer liefert, sondern den Index. Bei Nummer denken die meisten Leser die Zählung beginnt bei 1 und nicht bei 0 wie bei einem Index. Und dann sollten Funktionsnamen *Tätigkeiten* beschreiben, damit man sie leichter von eher passiven Werten unterscheiden kann.
Statt `trace` sollte man besser testbaren, sauberen Code schreiben. Funktionen bekommen alles was sie ausser Konstanten benötigen als Argument(e) übergeben.
Namen sollten keine kryptischen Abkürzungen enthalten, oder gar nur daraus bestehen.
Eine Funktion mit lokalen Namen wie 'arr', 'l', 'oc', 'ocm', 'ocmx', 'ocx', 'op', 'opx', 'out', 'ov', 'ovx', 'res', und 'x' versteht man nicht und das lädt auch zu schwer zu findenen Fehlern geradezu ein.
`arr` ist neben der Abkürzung auch kein sinnvoller Name für eine Liste. Listen sind keine Arrays, oder anders herum: es gibt in Python auch Arrays und man meint damit meistens Numpy-Arrays, darum ist das verwirrend Listen so zu nennen.
`subprocess.getoutput()` sollte man nicht verwenden weil das eine Shell zwischen das Programm und das externe Programm setzt, die Probleme machen kann auf die man gut verzichten kann. Wie man das konkret durch `subprocess.run()` ersetzt, hängt davon ab wie `rloader` aussieht.
Das ``rloader.replace("$", …)`` sieht so aus als wenn da Templating selbst programmiert, wofür es schon mehrere Lösungen fertig gibt. Im einfachsten Fall die `format()`-Methode auf Zeichenketten.
An der Stelle möchte man auch das auslesen eines einzelnen Laders aus der Funktion heraus ziehen, damit man das leicht einzeln testen kann.
Das ersetzen von Zeichenenden durch Leerzeichen ist unnötig weil `split()` ohne Argumente sowohl an Zeilenumbrüchen als auch an Leerzeichen aufteilt. An allen Whitespace-Zeichen um genau zu sein.
Es ist ineffizient die gleiche Zeichenkette immer wieder aufzuteilen um nur ein einzelnes Element zu nehmen und die Liste wieder zu verwerfen.
`ovx` wird potentiell gar nicht definiert.
Das ersetzen von Einheiten wie "V" und "A" sollte eher nicht mit `replace()` passieren, sondern mit `rstrip()`. Das beschreibt besser was da wo entfernt werden soll.
``int(0)`` ist unnötig kompliziert für einfach nur ``0``.
Bei der Leistung ist der `isnumeric()`-Test komisch. Einfach umwandeln und entsprechend reagieren wenn das auf die Nase fällt statt vorher einen Text zu machen der beim Umwandeln sowieso schon inklusive ist.
Statt eines Tupels würde ich hier einen eigenen Datentyp für das Ergebnis schreiben. Einmal damit die Komponenten einen sinnvollen Attributnamen bekommen, aber auch weil hier Addition von Ergebnissen und die Division durch einen skalaren Wert das Programm vereinfachen können.
Zwischenstand (ungetestet):
Code: Alles auswählen
import subprocess
from attr import attrib, attrs
@attrs(frozen=True)
class Output:
voltage = attrib(default=0)
current = attrib(default=0)
max_current = attrib(default=0)
power = attrib(default=0)
def __add__(self, other):
return Output(
self.voltage + other.voltage,
self.current + other.current,
self.max_current + other.max_current,
self.power + other.power,
)
def __div__(self, scalar):
return Output(
self.voltage / scalar,
self.current / scalar,
self.max_current / scalar,
self.power / scalar,
)
def read_loader(index):
try:
result = subprocess.run(
[
"the_command",
"--option",
str(index),
"--maybe-some-other-option",
],
stdout=subprocess.PIPE,
check=True,
text=True,
)
except subprocess.CalledProcessError:
#
# TODO Add some logging so searching for bugs would be easier/possible.
#
return Output()
words = result.stdout.split()
try:
try:
i = words.index("Output_Voltage")
except ValueError:
voltage = 0
else:
voltage = float(words[i + 1].rstrip("V"))
current = int(float(words[25].rstrip("A")))
max_current = int(float(words[27].rstrip("A")))
try:
power = int(float(words[32].rstrip("W")))
except ValueError:
power = 0
return Output(voltage, current, max_current, power)
except (IndexError, ValueError):
#
# TODO Add some logging so searching for bugs would be easier/possible.
#
return Output()
def read_loaders(loader_count):
return sum(map(read_loader, range(loader_count)), Output()) / loader_count
Das parsen der Ausgabe vom externen Programm kann man sicher etwas verständlicher und robuster mit regulären Ausdrücken machen. Und zum leichteren Testen würde man das auch besser in eine eigene Funktion herausziehen.
@noisefloor: Laut Ausgangsbeitrag kommt franze_m mit dem Debugger von PyCharm nicht mehr weiter — da wird pdb IMHO nicht besser sein.