@forumseeker: `helper` wird importiert, aber nirgends verwendet.
Auf Modulebene sollte nur Code stehen der Konstanten, Funktionen, und Klassen definiert. Das Hauptprogramm steht üblicherweise in einer Funktion die `main()` heisst.
Es macht wenig Sinn Funktionen wie `os.path.join()` zu verwenden und dann doch selbst noch wieder hart in den Code geschriebene Pfadtrenner zu verwenden, die nur unter Windows funktionieren. Und dann auch noch Pfadteile mit `+` zusammenzusetzen. Ausserdem verwendet man in neuem Code besser `pathlib` statt `os.path` & Co.
`time.time()` ist für Zeitmessungen nicht wirklich geeignet. Dafür gibt es `time.monotonic()`.
Sowohl `change` als auch `number` sind überflüssig. Wenn sich nichts geändert hat ist `number` leer und die Anzahl entspricht der Anzahl der Elemente in `number`. `number` sollte dementsprechend auch `numbers` heissen, denn das steht ja nicht für *eine* Zahl sondern für (potentiell) mehrere. Ausserdem sollte das eine Menge und keine Liste sein. Und es sollte nicht so verdammt weit vor der Schleife definiert werden in der das letztendlich verwendet wird. Das macht das Programm unübersichtlicher und schwerer Funktionen heraus zu ziehen, wenn die über den gesamten Code verstreut sind.
`numbers` könnte einen etwas aussagekräftigeren Namen bekommen.
`outfile_all` und `outfile` sind auch zu früh definiert. An der Stelle ist noch gar nicht entschieden ob die Werte überhaupt benötigt werden. `file` ist auch ungünstig als Name, denn bei einem `file` erwartet der Leser ein Dateiobjekt und keine Zeichenkette mit einem Pfad (oder ein `Path`-Objekt).
Bei der Auflistung der Dateien könnte/sollte man Verzeichnisse ausfiltern und/oder mit `glob()` nach Dateinamenserweiterungen filtern.
`MAX_ROW` und `MAX_COLUMN` sind keine Konstanten, sollten also auch nicht so geschrieben werden.
`time.strftime()` liefert eine Zeichenkette, da macht es keinen Sinn noch mal `str()` drauf los zu lassen.
`row_idx` und `col_idx` würde ich nicht Index nennen. Indexwerte fangen in Python immer bei 0 an, die Zeilen- und Spaltennummern aber bei 1.
Die temporäre Ausgabedatei würde ich auslassen, oder sind die Tabellen tatsächlich so gross, dass diese Informationen nicht in den Arbeitsspeicher passen?
Bei den beiden `open()` zum Schluss wird weder die Kodierung angegeben, noch werden die Dateien wieder geschlossen. Da aber mindestens mal bei „Geräte“ ein „ä“ vorkommt ist die Kodierungsangabe wichtig, und für das anschliessende Kopieren der Datei ist das schliessen wichtig, denn sonst sind da eventuell nicht alle Zeilen in der Datei.
`os.system()` sollte man nicht verwenden. Die Dokumentation verweist auf das `subprocess`-Modul. Aber auch das braucht man nicht um Dateien zu kopieren. Dafür gibt es in der Standardbibliothek etwas im `shutil`-Modul.
Zwischenstand (ungetestet):
Code: Alles auswählen
#!/usr/bin/env python3
import time
from pathlib import Path
from shutil import copy
import openpyxl
from openpyxl.formatting.formatting import ConditionalFormattingList
from openpyxl.styles import PatternFill
SELF_PATH = Path(__file__).absolute().parent
FOLDER_PATH = SELF_PATH / "Sicherung"
OUTPUT_FOLDER_PATH = SELF_PATH / "Results"
NAS_FOLDER_PATH = Path(R"N:\HA\Results")
MARKER_COLOR = PatternFill(fgColor="00008080", fill_type="solid")
DEVICES_SHEET_NAME = "Geräteliste"
def main():
start_time = time.monotonic()
#
# TODO Fällt unschön auf die Nase wenn nicht mindestens zwei Dateien
# vorhanden sind, ausserdem wäre ein glob() auf die Dateinamensendung
# vielleicht auch eine gute Idee.
#
old_file_path, new_file_path = sorted(
path for path in FOLDER_PATH.iterdir() if path.is_file()
)[-2:]
print(f"alte Datei: {old_file_path.name}")
print(f"neue Datei: {new_file_path.name}")
old_sheet = openpyxl.load_workbook(old_file_path, data_only=True)[
DEVICES_SHEET_NAME
]
new_workbook = openpyxl.load_workbook(new_file_path, data_only=True)
new_sheet = new_workbook[DEVICES_SHEET_NAME]
new_sheet.conditional_formatting = ConditionalFormattingList()
#
# TODO Fehlerbehandlung falls Spalte "B" komplett leer ist.
#
max_row = next(
cell for cell in reversed(new_sheet["B"]) if cell.value is not None
).row
changed_rows_numbers = set()
change_info_lines = []
for column_number in range(
3, max(old_sheet.max_column, new_sheet.max_column) + 1
):
for row_number in range(3, max_row + 1):
old_cell = old_sheet.cell(column=column_number, row=row_number)
new_cell = new_sheet.cell(column=column_number, row=row_number)
if old_cell.value != new_cell.value:
new_cell.fill = MARKER_COLOR
changed_rows_numbers.add(row_number)
change_info_lines.append(
f"Datei: {new_file_path.name}"
f" Spalte: {column_number},"
f" Zeile: {row_number},"
f" Änderung von: {old_cell.value}"
f" in: {new_cell.value}\n"
)
verstrichene_zeit = time.monotonic() - start_time
if changed_rows_numbers:
result_workbook_path = OUTPUT_FOLDER_PATH / (
time.strftime("%Y%m%d_%H%M%S") + "_results.xlsx"
)
new_workbook.save(result_workbook_path)
new_workbook.close()
change_info_file_path = OUTPUT_FOLDER_PATH / "results_all.txt"
change_info_lines.extend(
[
f"Änderung bei {len(changed_rows_numbers)} Geräten\n",
f"Ausführungszeit: {int(verstrichene_zeit)} Sekunden\n",
"\n",
]
)
with change_info_file_path.open("w", encoding="utf-8") as file:
file.writelines(reversed(change_info_lines))
for source_path in [result_workbook_path, change_info_file_path]:
copy(source_path, NAS_FOLDER_PATH)
if __name__ == "__main__":
main()
Nächster Schritt wäre es da sinnvoll Funktionen heraus zu ziehen. Die `main()` ist für meinen Geschmack zu umfangreich.
