Projekt eines Halb-Anfängers

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

Hallo!

Ich habe ein Programmierprojekt angefangen, um die Temperaturdaten des DWD, die als Zahlen vorliegen, zu visualisieren: https://gitlab.com/muessjoeh/temperaturverlauf
Ich bin zwar kein Anfänger in der Programmierung, aber sehr wohl in Python. Es kann also gut sein, dass ich ein paar Altlasten aus anderen Sprachen mitschleppe.
Könntet ihr mal bitte schauen, ob ihr irgendwelche Schnitzer findet?

Christian
Benutzeravatar
sparrow
User
Beiträge: 4164
Registriert: Freitag 17. April 2009, 10:28

@zegru: Was als erstes auffällt: du hast den Code auf seh viele Dateien/Module aufgeteilt. Gefühl eine Klasse je Modul. Das wirkt ein bisschen wie bei Java, wo man jede Klasse in einer Datei unterbringt. Das ist bei Python eher ungewöhnlich und macht das Lesen sehr kompliziert, weil man sich durch die Dateien hangeln muss.
Sirius3
User
Beiträge: 17710
Registriert: Sonntag 21. Oktober 2012, 17:20

@zegru: noch zu den vielen Dateien: alle Python-Dateien sollten eigentlich in einem Package stehen und nicht als viele Module auf oberster Ebene.
Module werden wie Variablennamen oder Funktionen komplett klein geschrieben. Eine Datei pro Klasse macht man nicht, da damit ja der Namespace, den ein Modul aufspannt, gar nicht verwendet wird, als überflüssig wäre.
Wenn ich in ein paar Dateien reinschaue, sind das oft nichtmal richtige Klassen.
`AccessLocation` hat als einziges Attribut die Session, die sowieso nicht in einer Klasse gespeichert werden sollte, da damit der Session-Pool von SQL-Alchemy ausgehebelt wird. Damit hast Du nur drei Funktionen, die auf die Session aufsetzen, und keine Klasse. In `get_location` holst Du Dir sogar nochmal eine neue Session.
Ohje, get_session holt sich gar keine neue Session, sondern erzeugt jedesmal einen neuen Session-Pool.
Dann gibt es Module, die nicht mal so existieren dürften. Das was in base steht oder in logger sind eigentlich Dinge, die am Anfang eines Programms initialisiert werden, und nicht schon, wenn ein Modul importiert wird.
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

Das meinte ich mit "Altlasten". Ich habe nämlich tatsächlich in Java programmiert. Wie wird das denn in Python konkret gehandhabt, auch mit der Handlichkeit von größeren Dateien?
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

Bei der 'AccessLocation' muss ich Dir Recht geben: Die Klasse ist immer weiter geschrumpft, bis man das, was noch übrig ist, direkt hinschreiben kann.
Was 'base' angeht: Ich wollte ein Singleton haben. Gibt es unter Python einen anderen Weg, der vorzuziehen ist?
'connection': Ich wollte es für den Entwickler einfach machen, zwischen verschiedenen RDBMS zu wechseln. Kann man dies auch auf einen anderen, üblicheren Weg erreichen?
'logger': Es passieren dort ja mehr Dinge, als lediglich einen Logger zu holen.

Generell kann ich feststellen, dass in der Programmierung in Python weniger Klassen verwendet werden als bei den Programmiersprachen, die ich bis jetzt gewohnt bin. Gibt es noch andere typische Wechsler-Fehler, die ich mache?
__deets__
User
Beiträge: 14493
Registriert: Mittwoch 14. Oktober 2015, 14:29

Singletons habe ich schon lange keines mehr nutzen müssen. Auch in anderen Sprachen nicht. Sowas erzeugt immer schlechte testbarkeit und zu enge Kopplung. Warum denkst du das du das brauchst?
Benutzeravatar
__blackjack__
User
Beiträge: 13003
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@__deets__: Ich weiss nicht ob Du reingeschaut hast: da wird per `declarative_base()` von `sqlalchemy` die Basisklasse für die ORM-Modelklassen erstellt. Das macht man halt so. Was ich aber nicht verstehe ist warum das im Grunde als einzige Zeile in einem eigenen Modul steht.

@zegru: Re `connection`: Da hätte ich gesagt dafür benutzt man am besten `sqlalchemy`. Was Du ja schon tust. Und ich dann neugierig in diese Datei geschaut habe um zu sehen was Du da wohl meinen könntest: Eine Funktion `connection_url()` die einfach nur eine Connection-URL zurück gibt die da literal drin steht. Ähm, üblich ist es das so etwas kein Code ist, sondern Konfiguration, also Daten.

Die Anmerkung zum `logger`-Modul verstehe ich nicht. Das ist kein Grund das a) in ein eigenes Minimodul zu stecken und b) dass das einfach so beim importieren abläuft. Zudem ist `__name__` dort komisch, denn für den Logger eines Programms würde man nicht den Namen eines so kleinen Hilfsmodul erwarten, sondern den Namen des Programms. Was wieder ein Zeichen ist, dass das ins Hauptprogramm gehört.

Ich habe das Projekt nicht runtergeladen und nicht in alle Dateien reingeschaut, aber in `persist.py` passiert auch wesentlich zu viel wenn man das Modul einfach nur importiert. Sollte so etwas auch in anderen Modulen der Fall sein: in Funktion(en) verfrachten und per ``if __name__ == "__main__":`` absichern.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
__deets__
User
Beiträge: 14493
Registriert: Mittwoch 14. Oktober 2015, 14:29

@__blackjack__ natürlich nicht 🤭Ich sollte einfach nix mehr posten, weil ich meistens am iPad hier bin - und da Code schreiben und gut reviewen so seine Tücken hat.
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

@alle Ich habe ein neues Feature eingebaut, was leider so halb fertig gar nicht funktionierte, und das ich erst fertig programmieren musste. Man kann git zwar so
einsetzen, dass andere Projekte wie z.B. Reviews einzupflegen einfach möglich sind, leider bin ich aber auch ein Umsteiger, was verteilte Versionskontrollsysteme angeht. Deshalb die Verzögerung beim Antworten. Generell ist aber konstruktive Kritik bei mir sehr gerne gesehen, schließlich will ich ja keine unpassenden Gewohnheiten aus anderen Programmiersprachen übernehmen. Ich habe auch etliche Vorschläge in Code gegossen.

Ich glaube, dass es zu umständlich ist, einzeln zu antworten, deshalb erfolgt hier ein Kommentar an alle:
Wegen 'Base': Ich hatte es ebenfalls aus historischen Gründen wie 'AccessLocation' in einer eigenen Datei. Nun habe ich die Definition aber direkt bei der Definition der Mapping Klassen.
Wegen 'connection': Ich habe nun die connection_url in einer Variable und nicht in einer Funktion.
Wegen 'logger': Dieses Modul wird quasi in jeder Python-Datei importiert. Darin wird nicht nur der Logger geholt, sondern auch der von SQLAlchemy weniger geschwätzig geschaltet. Gibt es eine schlauere Art, dies zu bewerkstelligen?
Benutzeravatar
__blackjack__
User
Beiträge: 13003
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@zegru: Konfigurieren vom Logging macht man in der Regel in dem Modul wo das Programm drin steht. Und man nimmt nicht überall den gleichen Logger sondern erstellt sich einen pro Modul mit dem Namen (`__name__`) des Moduls. Dann hat man auch gleich eine Loggerhierarchie die der Modulhierarchie entspricht.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

Ich habe nun den Logger pro Modul definiert und feile an ihm per import herum. Gibt es sonst noch Python-untypische Codeteile?
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

Und außerdem verschiedene Klassen jeweils in ein einziges Modul zusammengefasst.
Benutzeravatar
__blackjack__
User
Beiträge: 13003
Registriert: Samstag 2. Juni 2018, 10:21
Wohnort: 127.0.0.1
Kontaktdaten:

@zegru: Als grösse Änderung würde ich da ja noch die ganzen Module in einem Package zusammenfassen, damit die nicht mit was auch immer so installiert ist im gleichen Namensraum konkurrieren. Man sieht dann auch leichter/schneller was die Programme sind. Wobei ich persönlich Programme auch in der Regel ins Package stecke und entweder nur kleine Skripte schreibe die das dann dort ausführen, beziehungsweise Entry-Points verwende damit die `setup.py` diese Skripte (und unter Windows *.EXEn) für mich erstellt.

`xopen()` ist IMHO ein Problem weil Du da weder die Kodierung noch das `newline`-Argument angeben kannst.

Ich hatte mir jetzt erst einmal die `persist.py` als Einstiegspunkt angeschaut: Da ist eine ziemlich wilde Mischung aus `print()`, `print()` auf `sys.stderr`, und Logging. Mindestens mal die Ausgaben auf `stderr` sind ja eigentlich Logging-Ausgaben auf DEBUG-Level. Aber auch die normalen `print()`-Ausgaben könnte man als INFO-Level über Logging lösen. Und DEBUG-Logging-Aufrufe auskommentieren? Genau deswegen benutzt man doch Logging statt `print()`, damit man die Ausgaben über Level regeln kann ohne den Code anfassen zu müssen.

`readline()` ist eine Methode die man eher nicht verwendet. Und im Zusammenhang mit einer ``while True:``-Schleife um über Zeilen einer Datei zu iterieren schon gar nicht. Dateiobjekte sind Iteratoren über die Zeilen, da kann man also die `next()`-Funktion für einzelne Zeilen verwenden und eine ``for``-Schleife über die Zeilen.

Wenn etwas `irgenwas_file` heisst, erwartet der der Leser ein Dateiobjekt und keinen Dateipfad. Und wenn man ein `Path`-Objekt hat, macht es keinen Sinn das dann zusätzlich in eine Zeichenkette zu wandeln und mit Zeichenkettenoperationen darauf zu operieren.

`Session.add_all()` ist die falsche Methode wenn man damit immer genau *ein* Objekt zur Sitzung hinzufügt.

Das filtern nach Dateinamensbestandteilen sieht insgesamt sehr fragil aus. Da würde man besser Unterordner in data/ anlegen wo die verschiedenen Dateitypen einsortiert werden und eine README in data/ die erklärt was wo rein gehört und das da nix anderes rein gehört.

Die Module `connection` und `logger` sind IMHO zu dürftig. Konstanten schreibt man KOMPLETT_GROSS. Und irgendwie stehen da auch Sachen drin die in eine Konfigurationsdatei gehören. Die dann auch nicht unbedingt in das Repository gehört. Jedenfalls nicht (komplett) ausgefüllt, denn so etwas wie Zugangsdaten zur Datenbank gehört nicht in ein Repository.
“Most people find the concept of programming obvious, but the doing impossible.” — Alan J. Perlis
zegru
User
Beiträge: 49
Registriert: Freitag 9. Oktober 2020, 09:22

@alle Zuerst mal ein dickes Danke!

Als Anfänger in der Programmiersprache Python wollte ich ja verhindern, dass ich Muster aus anderen Programmiersprachen übernehme. Damit konnte ich dank eurer Hilfe einen Anfang machen!

> @zegru: Als grösse Änderung würde ich da ja noch die ganzen Module in einem Package zusammenfassen, damit die nicht mit was auch immer so installiert ist im gleichen Namensraum konkurrieren. [...]

Wenn die eigenen Prozeduren und Klassen in einem eigenen Namensraum sind, ist das sicherlich erstrebenswert. Wie macht man das denn konkret? Eine zusätzliche Verzeichnisebene einziehen?

> `xopen()` ist IMHO ein Problem weil Du da weder die Kodierung noch das `newline`-Argument angeben kannst.

Stimmt, in dem Fall wäre es doof. Ich habe das mit der Transparenz nun so gelöst, dass aufgrund des Dateinamens entschieden wird (mir ist auch noch nie eine Datei untergekommen, die falsch benannt ist)

> Ich hatte mir jetzt erst einmal die `persist.py` als Einstiegspunkt angeschaut: Da ist eine ziemlich wilde Mischung aus `print()`, `print()` auf `sys.stderr`, und Logging. Mindestens mal die Ausgaben auf `stderr` sind ja eigentlich Logging-Ausgaben auf DEBUG-Level. Aber auch die normalen `print()`-Ausgaben könnte man als INFO-Level über Logging lösen.

Als ich mir 'persist.py' vorgenommen habe, habe ich mir gedacht: Oje, Spaghetticode! Die Änderungen, um Python-Phrasen zu verwenden, haben die Strukturierung jedoch stark verbessert. Übrigens, Respekt, wer sich anderer Leute Spaghetticode vornimmt!
Die einzelnen Ausgaben haben ja verschiedene Aufgaben: Logging für den Entwickler, Ausgaben für den Benutzer und eine Fortschrittsanzeige. Streckenweise waren diese Aufgaben verwurschtelt. Vor allem die Fortschrittsanzeige habe ich nun mit der Library 'tqdm' gelöst, was den Code *wesentlich* leichter les-, schreib- und wartbar gemacht hat.

> Und DEBUG-Logging-Aufrufe auskommentieren? Genau deswegen benutzt man doch Logging statt `print()`, damit man die Ausgaben über Level regeln kann ohne den Code anfassen zu müssen.

Jein. Wenn die Logging-Aufrufe einerseits innerhalb einer Schleife sind und andererseits ausserhalb, sieht man streckenweise den Wald vor lauter Bäumen nicht mehr.

> `readline()` ist eine Methode die man eher nicht verwendet.

Ich habe mal danach gegoogelt, wie man in Python eine Datei einliest, und habe mir das mit readline() gemerkt. Allerdings ist das mit der for-Schleife viel verständlicher und intuitiver! Ich habe es nun entsprechend umgestellt.

> Wenn etwas `irgenwas_file` heisst, erwartet der der Leser ein Dateiobjekt und keinen Dateipfad.[...]

Ich habe die Variablen nun entsprechend umbenannt.

> `Session.add_all()` ist die falsche Methode wenn man damit immer genau *ein* Objekt zur Sitzung hinzufügt.

Auch diesen Aufruf habe ich geändert.

> Das filtern nach Dateinamensbestandteilen sieht insgesamt sehr fragil aus.

Das Filtern war zwar nicht fragil, aber mein Verständnis. Ich habe dies nun geändert. Und es ist dadurch wesentlich intuitiver.

> Die Module `connection` und `logger` sind IMHO zu dürftig.

Diese Module werden öfter eingebunden und erledigen noch andere Dinge, wie beispielsweise den Logger in manchen Bereichen weniger geschwätzig zu machen, oder eine Session aus SQLAlchemy holen. Wie würdest Du das konkret bewerkstelligen, ohne die Redundanz zu erhöhen?

> Konstanten schreibt man KOMPLETT_GROSS.

Ich habe dies nun entsprechend geändert.

> Und irgendwie stehen da auch Sachen drin die in eine Konfigurationsdatei gehören. Die dann auch nicht unbedingt in das Repository gehört. Jedenfalls nicht (komplett) ausgefüllt, denn so etwas wie Zugangsdaten zur Datenbank gehört nicht in ein Repository.

Das hat mich auch schon intuitiv gestört, ohne dass ich auf mein schlechtes Gefühl gehört hätte. Ich verwende nun eine Konfiurationsdatei im JSON Format.
Antworten