Code Style Review

[Marc Haber]

Hallo,

ich wollte mich schon seit Jahren mal etwas ernsthafter mit python
beschäftigen und hier ist mein Erstlingswerk signifikanter Länge.
pylint hat nichts auszusetzen, an manchen Stellen habe ich pylint aber
überstimmt.

Ist da irgendetwas drin, was nicht pythonesk genug formuliert ist?

Laufen tut das Programm jedenfalls.

Ich würde mich über Eure Kommentare freuen.

Grüße
Marc



#!/usr/bin/python
"""
Monitor the MQTT messages from an electric meter
and send out information when an applicance has started and ended
its run
"""

import sys
import argparse
import json
import time
import pprint
import threading
import yaml
import paho.mqtt.client as mqtt

pp = pprint.PrettyPrinter(indent=4)

argparser=argparse.ArgumentParser(
prog = 'appliance',
description = 'monitor appliance power consumption and'
+ 'send status messages',
)
argparser.add_argument('-c',
'--config',
help='path to configuration file',
)
argparser.add_argument('-d',
'--debug',
action='store_true',
help='activate debugging',
)
args=argparser.parse_args()

# pylint issue 2166: all module-level assignments get flagged with
this
# how would I properly define a global variable?
# pylint: disable-next=invalid-name
debug = 0
if args.debug:
# pylint: disable-next=invalid-name
debug = 1

if debug > 0:
print('parsed arguments:')
pp.pprint(args)

try:
with open(args.config, encoding="utf-8") as c:
config=yaml.safe_load(c)
except FileNotFoundError:
sys.exit('configuration file '+args.config+' not found or
unreadable')

if debug > 0:
print('read config:')
pp.pprint(config)

if 'debug' in config:
debug = config['debug']

table=[
{
'timestamp': time.time(),
'leistungRounded': 0,
'leistung': 0,
}
]

# pylint: disable-next=invalid-name
status = 'inaktiv'
status_change = time.time()

# pylint:
disable-next=redefined-outer-name,unused-argument,invalid-name
def on_connect(client, userdata, flags, rc):
"""handle successful connect callback"""
if debug > 0:
print('Connected to ',
config['config']['host'],
'port',
config['config']['port'],
'with result code',
str(rc),
)
print('subscribe to topic '+config['config']['topic'])

client.subscribe(config['config']['topic'])

def print_table():
""" print table for debugging """
if debug > 0:
now=time.time()
for element in table:
print(int(now-element['timestamp']),
element['leistungRounded'],
round(element['leistung'],6),
element.get('meter',''),
)
print('length:',
len(table),
'max:',
max_leistung(table),
)
print('-----------')

cleanuplock=threading.Lock()
def cleanup_table():
"""clean up table, removing data that is older than cutoff time"""
with cleanuplock:
while (
len(table) > 1
and table[0]['timestamp']+config['config']['cutoff_time']
< time.time()
):
if debug > 0:
print('remove from beginning')
table.pop(0)

def add_element(newdata):
"""create new data record and add to table"""
rounded = int(newdata['leistung']*1000)
element={
'leistung': newdata['leistung'],
'leistungRounded': rounded,
'timestamp': time.time(),
}
if 'meter' in newdata:
element['meter']=newdata.get('meter')
table.append(element)
if debug > 0:
print(
'add:',
int(element['timestamp']),
element['leistungRounded'],
round(element['leistung'],6),
)

# pylint: disable-next=redefined-outer-name
def max_leistung(table):
"""return maximum "leistung" found in table if recent enough"""
maxvalue=0
elementfound=0
for element in table:
if (
element['timestamp']+config['config']['cutoff_time']
< time.time()
):
continue
elementfound=1
if element['leistungRounded']>maxvalue:
maxvalue=element['leistungRounded']
if elementfound==0:
maxvalue=table[len(table)-1]['leistungRounded']
return maxvalue

# pylint: disable-next=redefined-outer-name↲
def report_status_change(status):
"""report status change. Plug in messenger code here"""
print('statuswechsel:',
config['config']['friendlyname'],
'ist',
status,
)

evaluatelock=threading.Lock()
def evaluate():
"""
Evaluate the current table, handle internal state machine and
call function to act on relevant status change
"""
# pylint: disable-next=global-statement,invalid-name
global status
with evaluatelock:
maxl=max_leistung(table)
if (
status=='fertig'
and status_change+config['config']['inactive_time'] <
time.time()
):
status='inaktiv'
if status=='aktiv':
if maxl < config['config']['on_threshold']:
status='fertig'
report_status_change(status)
if (status!='aktiv' and maxl >=
config['config']['on_threshold']):
status='aktiv'
report_status_change(status)

# pylint: disable-next=unused-argument,redefined-outer-name
def on_message(client, userdata, msg):
"""
Callback function acting on an incoming MQTT message
"""
if debug > 0:
print('receive MQTT message to',
msg.topic,
':',
str(msg.payload),
)
jsonobject=json.loads(msg.payload)
cleanup_table()
add_element(jsonobject)
if debug > 0:
print_table()
evaluate()

client = mqtt.Client()
client.on_connect = on_connect
client.on_message = on_message

client.connect(config['config']['host'], config['config']['port'], 60)

client.loop_start()

while True:
time.sleep(30)
if debug > 0:
print('run evaluate() and cleanup_table() from main loop')
cleanup_table()
if debug > 0:
print_table()
evaluate()

# end of file
--
-------------------------------------- !! No courtesy copies, please !! -----
Marc Haber | " Questions are the | Mailadresse im Header
Mannheim, Germany | Beginning of Wisdom " |
Nordisch by Nature | Lt. Worf, TNG "Rightful Heir" | Fon: *49 621 72739834

[c.b...@posteo.jp]

Hallo Marc,

erst einmal finde ich es super, dass du dich so intensiv damit
auseinandersetzt.

Die Liste ist für ein Code Review etwas ungeeignet. Vielleicht kannst du
den Code auf Codeberg.org oder wo anders hosten, dann lässt er sich
leichter kommentieren.

Neben PyLint könntest du auch nochmal "flake8" auf den Code loslassen.

Die Frage ist, warum du solche PyLint Ausnahmen ("# pylint:") definiert
hast. IMHO benötigt man dafür einen wirklich triftigen Grund; eine
strenge Indikation. Die Hinweise und Meldungen von Lintern haben ihre
Berechtigung und sollten nicht unterdrück werden.

Neben den Lintern würde ich dir noch empfehlen, ein paar mehr Leerzeilen
zu machen. Das erhöht IMHO die Lesbarkeit, gerade bei verschachtelten
Strukturen. Natürlich ist das Geschmacksache bzw. eine Frage des
Team-Konsent. Z.B. hier:

[Christopher Arndt]

Am 28.11.22 um 16:37 schrieb c.b...@posteo.jp:

> Neben PyLint könntest du auch nochmal "flake8" auf den Code loslassen.

Außerdem könntest du auch noch entweder "black" oder "yapf" oder ein
ähnliches Tool benutzen, um den Code einheitlich und anhand üblicher
Gepflogenheiten zu formatieren.

Weiterhin:

- Sortiere die Imports mit "isort"
- Strukturiere den Code in Sektionen. Vorschlag

- Module docstring
- Imports
- Global constants and variables
- Exceptions
- Classes
- Helper functions
- main function

Momentan sind bei dir Code auf globaler Ebene und Funcktionen wild
gemischt. Das ist ziemlich unübersichtlich und verleitet m.E. dazu, den
angemessenen "Separation of concern" aus dem Auge zu verlieren. Auf
globaler Ebene würde ich idR wirklich nur einige wichtige Konstanten
definieren und den Rest in eine "main"-Funktion verschieben. Diese wird
dann am Ende des Scripts aufgerufen mit:

if __name__ == '__main__':
main()

Das zwingt dazu, sich überlegen zu müssen, welche Daten wirklich global
sein müssen und wie ansonsten Daten zwischen Funktionen und Klassen
weitergereicht werden.


Chris

[c.b...@posteo.jp]

Eigentlich wollte ich dieses Thema nicht anschneiden, um den Unmut der
Liste nicht auf mich zu ziehen. ;)
Ich versuche es vorsichtig.

Ich hatte Eingangs ja das Lob ausgesprochen, dass er sich mit dem Thema
so bewusst und intensiv auseinandersetzt. So lernt man nicht nur,
sondern verbessert auch seine Skills.

Am 28.11.2022 16:48 schrieb Christopher Arndt:
> Außerdem könntest du auch noch entweder "black" oder "yapf" oder ein
> ähnliches Tool benutzen,

> [..]

> - Sortiere die Imports mit "isort"

Die hier genannten automatisierenden Tools verhindern dieses Lernen.
Gerade Anfängern möchte ich dringend von solchen Tools abraten. Sorge
selbst dafür, dass dein Code sauber ist. Überprüfe deinen Code mit
Lintern, aber das Korrigieren bitte selbst machen. Linter kann man auch
live in einer IDE dazuschalten, so dass du schon beim Tippen durch
Hervorhebungen die Mitteilung erhältst, dass du da etwas besser machen
könntest.

Du möchtest sauberen Code im Repo haben und du möchtest alle Leute
deines Teams dazu bringen nur sauberen Code zu committen?
Dann mach einen pre-commit-hook oder einen Unittest der Alarm schlägt,
wenn ein Linter wie pylint/flake8 Fehler wirft. Für kleinere Projekte
wähle ich gerne die unittest Variante (Ein sicher verbesserungswürdiges
Beispiel: [1]).

[1] --
<https://codeberg.org/buhtz/hyperorg/src/commit/463ca3607943f9d6651d08aa3f35a95177d493c6/tests/test_content.py#L13-L19>

[Marc Haber]

c.b...@posteo.jp wrote:
>Neben PyLint könntest du auch nochmal "flake8" auf den Code loslassen.

Das war eine gute Anregung, vim-ale hat das direkt aufgegriffen und
noch ein paar Anregungen im Code hinterlassen.

>Die Frage ist, warum du solche PyLint Ausnahmen ("# pylint:") definiert
>hast. IMHO benötigt man dafür einen wirklich triftigen Grund; eine
>strenge Indikation. Die Hinweise und Meldungen von Lintern haben ihre
>Berechtigung und sollten nicht unterdrück werden.

Das sehe ich mindestens teilweise anders. Wenn ich irgendwo explizit
hinschreibe, dass eine Funktion eine globale Variable verwendet (wie
z.B. das dictionary, in dem die Konfiguration drin steht) und der
Linter das unkonditional anmeckert nur allein weil das Schlüsselwort
benutzt wurde ergibt das für mich keinen Sinn: Wenn ich das nicht
gewollt hätte hätte ich es nicht hingeschrieben.

Gerade für die Konfiguration halte ich die globale Variable auch für
den vernünftigen Weg, das wirklich überall bekannt zu machen. Die
Alternative, jeder Funktion das Konfig-Dictionary als Parameter
einzufüllen, finde ich noch schlechter lesbar. Wie ist in Python das
normale Idiom für sowas?

Und das hier:

|debug = 0
|if args.debug:

| debug = 1
zu flaggen mit "ey, debug ist eine Konstante, schreib die gefälligst
groß" finde ich einen echten Bug. Dass das keine Konstante ist, ergibt
sich doch direkt aus dem Code.

>Neben den Lintern würde ich dir noch empfehlen, ein paar mehr Leerzeilen
>zu machen. Das erhöht IMHO die Lesbarkeit, gerade bei verschachtelten
>Strukturen. Natürlich ist das Geschmacksache bzw. eine Frage des
>Team-Konsent.

Ja, das ist eine Geschmackssache, ich habe lieber mehr Informationen
auf dem immer zu kleinen Bildschirm. Aber das mag sich ändern wenn ich
mich in der Sprache ein bissche mehr daheim fühle.

>while True:
>
> time.sleep(30)
>
> if debug > 0:
> print('run evaluate() and cleanup_table() from main loop')
>
> cleanup_table()
>
> if debug > 0:
> print_table()
>
> evaluate()

Dafür habe ich noch nicht genug python-Auge um zu sehen dass das alles
zusammen gehört (meinen Augen fehlt die schließende Klammer, aber das
ist halt so). Vermutlich stimme ich Dir da in einem halben Jahr zu.

Grüße
Marc

[Marc Haber]

Christopher Arndt <ch...@chrisarndt.de> wrote:
>Am 28.11.22 um 16:37 schrieb c.b...@posteo.jp:
>> Neben PyLint könntest du auch nochmal "flake8" auf den Code loslassen.
>
>Außerdem könntest du auch noch entweder "black" oder "yapf" oder ein
>ähnliches Tool benutzen, um den Code einheitlich und anhand üblicher
>Gepflogenheiten zu formatieren.

black wird von flake8 und vim-ale direkt aufgegriffen, das ist sicher
auch eine Hilfe.

>Momentan sind bei dir Code auf globaler Ebene und Funcktionen wild
>gemischt. Das ist ziemlich unübersichtlich

... und perlish. Ich hab das mal angepasst auf

>if __name__ == '__main__':
> main()
>
>Das zwingt dazu, sich überlegen zu müssen, welche Daten wirklich global
>sein müssen und wie ansonsten Daten zwischen Funktionen und Klassen
>weitergereicht werden.

Da muss ich meine Logik noch etwas aufmischen weil das dann natürlich
nicht mehr mit den globals passt, da muss ich dann mit mehr
Funktions-Parametern arbeiten. Dazu melde ich mich in ein paar Tagen
wieder.

Kann ich die Locks innerhalb der entsprechenden Funktion definieren
oder bekomme ich dann in jedem Thread ein eigenes Lock, was der
Intention entgegen spricht?

Ist:

foolock = threading.Lock()
def foo:
with foolock:
(tue Dinge mit foo, potenziell multithreaded)

dasselbe wie

def foo:
foolock = threading.Lock()
with foolock:
(tue Dinge mit foo, potenziell multithreaded)

?

Ist die in
https://www.bogotobogo.com/python/Multithread/python_multithreading_Using_Locks_with_statement_Context_Manager.php
und
https://www.pythontutorial.net/python-concurrency/python-threading-lock/
verwendete Schreibweise, wo das Lock als Parameter mit in die Funktion
hineingereicht wird, wirklich "schönes" Python?

Oder schreibe ich mein ganzes Programm als eine Klasse, die genau
einmal instanziiert wird und dann in ihren statischen Variablen (so
wäre es in C++) die Tabelle, die Konfiguration, die Locks, den
Debugstatus etc hält? Sind solche Ein-Instanz-Klassen "schönes"
Python?

Grüße
Marc

[c.b...@posteo.jp]

Am 28.11.2022 21:52 schrieb Marc Haber:
> Das sehe ich mindestens teilweise anders. Wenn ich irgendwo explizit
> hinschreibe, dass eine Funktion eine globale Variable verwendet (wie
> z.B. das dictionary, in dem die Konfiguration drin steht) und der
> Linter das unkonditional anmeckert nur allein weil das Schlüsselwort
> benutzt wurde ergibt das für mich keinen Sinn

Auch wenn es radikal klingen mag; Es hat keinen Sinn "global" zu
verwenden, auch wenn Python das im Sprachumfang hat. Ein "global" hat
sehr viele Nebeneffekte und birgt potentielle Risiken, die den Gewinn
nicht aufwiegen. Es gibt andere Lösungen.

> Gerade für die Konfiguration halte ich die globale Variable auch für
> den vernünftigen Weg, das wirklich überall bekannt zu machen.

Nein, ist es nicht. Das ist nur eine "Abkürzung". Besser wäre hier eine
Art Singleton-Pattern anzuwenden.
Sagen wir deine Konfiguration ist in der Klasse "Config".

class Config:
@classmethod
def instance(cls):
# Provide the instance if it exists
if cls._instance:
return cls._instance

# But don't created implicit when needed.
raise Exception(f'No instance of class "{cls}" exists. '
'Create an instance first.')

def __init__(self):
# Exception when an instance exists
if __class__._instance:
raise Exception(
f'Instance of class "{self.__class__.__name__}" still
exists!'
f' Use "{self.__class__.__name__}.instance()" to access
it.')

# Remember the instance as the one and only singleton
__class__._instance = self

Quelle:
https://github.com/bit-team/backintime/blob/buhtz_config-singleton/common/config.py

[Enrik Berkhan]

c.b...@posteo.jp wrote:
> Am 28.11.2022 21:52 schrieb Marc Haber:
>> Gerade für die Konfiguration halte ich die globale Variable auch für
>> den vernünftigen Weg, das wirklich überall bekannt zu machen.
>
> Nein, ist es nicht. Das ist nur eine "Abkürzung". Besser wäre hier eine
> Art Singleton-Pattern anzuwenden.

Ich mag das Borg Pattern:

https://www.oreilly.com/library/view/python-cookbook/0596001673/ch05s23.html

Gruß,
Enrik

[Marc Haber]

c.b...@posteo.jp wrote:
>Am 28.11.2022 21:52 schrieb Marc Haber:
>> Das sehe ich mindestens teilweise anders. Wenn ich irgendwo explizit
>> hinschreibe, dass eine Funktion eine globale Variable verwendet (wie
>> z.B. das dictionary, in dem die Konfiguration drin steht) und der
>> Linter das unkonditional anmeckert nur allein weil das Schlüsselwort
>> benutzt wurde ergibt das für mich keinen Sinn
>
>Auch wenn es radikal klingen mag; Es hat keinen Sinn "global" zu
>verwenden, auch wenn Python das im Sprachumfang hat. Ein "global" hat
>sehr viele Nebeneffekte und birgt potentielle Risiken, die den Gewinn
>nicht aufwiegen. Es gibt andere Lösungen.

Und die wären? Jeder einzelnen Funktion in jedem einzelnen Aufruf das
config-Hash als Parameter mitgeben?

>> Gerade für die Konfiguration halte ich die globale Variable auch für
>> den vernünftigen Weg, das wirklich überall bekannt zu machen.
>
>Nein, ist es nicht. Das ist nur eine "Abkürzung". Besser wäre hier eine
>Art Singleton-Pattern anzuwenden.
>Sagen wir deine Konfiguration ist in der Klasse "Config".
>
>class Config:
> @classmethod
> def instance(cls):
> # Provide the instance if it exists
> if cls._instance:
> return cls._instance
>
> # But don't created implicit when needed.
> raise Exception(f'No instance of class "{cls}" exists. '
> 'Create an instance first.')
>
> def __init__(self):
> # Exception when an instance exists
> if __class__._instance:
> raise Exception(
> f'Instance of class "{self.__class__.__name__}" still
>exists!'
> f' Use "{self.__class__.__name__}.instance()" to access
>it.')
>
> # Remember the instance as the one and only singleton
> __class__._instance = self

Das sind 22 Zeilen für was? Und da habe ich immer noch nicht das
Problem gelöst, dass ich das einzelne Objekt dieser Klasse immer noch
als globale Variable mitziehen muss oder jede einzele Funktion in
jedem einzelnen Aufruf dieses Objekt als Parameter braucht.

Oder stehe ich hier auf dem Schlauch?

[c.b...@posteo.jp]

Hallo Marc,

Am 29.11.2022 15:22 schrieb Marc Haber:

> c.b...@posteo.jp wrote:
>> Es gibt andere Lösungen.
>
> Und die wären?

Siehe unten.

> Jeder einzelnen Funktion in jedem einzelnen Aufruf das
> config-Hash als Parameter mitgeben?

Nein, das würde ich auch nicht machen wollen. ;)

>> Besser wäre hier eine
>> Art Singleton-Pattern anzuwenden.
>> Sagen wir deine Konfiguration ist in der Klasse "Config".
>>
>> class Config:

>> [...]

> Das sind 22 Zeilen für was?

Bitte Pattern nicht als fertige Lösung betrachten, schon gar nicht im
Kontext von Python. Ich verstehe die immer als Lösungsansätze, die
sozusagen zum Denken anregen sollen. Sie müssen nicht 1 zu 1 umgesetzt
werden.

Es gab da ja auch den, für mich auch neuen, Vorschlag zum "Borg
Pattern".

> Und da habe ich immer noch nicht das
> Problem gelöst, dass ich das einzelne Objekt dieser Klasse immer noch
> als globale Variable mitziehen muss

Ne, eben nicht. Die Klasse kennt ihre eigene Instanz. Du musst dir keine
Gedanken mehr darum machen, "wo" diese Instanz herkommt.
Mach einfach "Config.instance()".

Eine andere Alternative wäre ein ganzes "config" Modul anzulegen; also
"config.py" die du mit "import config" bekannt machst und dann z.B.
"config.instance.getFoobar()" schreiben kannst. Deine globale Variable
kapselst du in einem Modul.

> Oder stehe ich hier auf dem Schlauch?

Wie gesagt, kann man Pattern (z.B. Singleton) genau dazu nutzen, um mal
"anders zu denken". Mir geht das bei dem "Borg" Ding so. Kannte ich
nicht. Finde ich auch nicht sehr geil. Aber erweitert den Horizont,
besonders wenn man sich die dahinterstehenden Prinzipien genau anschaut.

[Stefan Schwarzer]

Hallo Marc,

On 2022-11-28 16:19, Marc Haber wrote:
> ich wollte mich schon seit Jahren mal etwas ernsthafter mit python
> beschäftigen und hier ist mein Erstlingswerk signifikanter Länge.
> pylint hat nichts auszusetzen, an manchen Stellen habe ich pylint aber
> überstimmt.
>
> Ist da irgendetwas drin, was nicht pythonesk genug formuliert ist?
>
> Laufen tut das Programm jedenfalls.
>
> Ich würde mich über Eure Kommentare freuen.

ich habe hier und da ein paar Anregungen. Wie immer gilt,
dass letztlich der Autor entscheidet, wie er/sie diese
Anregungen umsetzt. :-)

Erst mal zum Thema der Konfigurations-Speicherung, was in
dem Diskussions-Thread auch aufgegriffen wurde. Da in Python
Module und Klassen natürliche Singletons sind, wäre mein
Vorschlag, die Konfigurations-Daten in einer Klasse zu
speichern, also

```
class config:

# Listener IP address and port of MQTT server.
ip_address = None
port = None

# If `True`, print debugging output.
debug = False

...
```

Du kannst einfach von überall aus dem Modul auf das
`config`-Objekt zugreifen und brauchst auch kein `global`.
Die Klasse wird nie instanziiert, sondern nur als Namespace
benutzt.

Der Ansatz hat gegenüber dem `config`-Dictionary auch den
Vorteil, dass du dokumentieren kannst, wofür die einzelnen
Werte gedacht sind. Ich halte es für gut, in `config` alle
Werte auf Default-Werte zu setzen, selbst wenn diese mangels
eines sinnvollen Defaults erst mal `None` sind. Das hat den
Vorteil, dass man sehen kann, was jemals gesetzt wird und
reduziert die Verwirrung, weil man nicht zwischen `None` und
"gar nicht gesetzt" unterscheiden muss. (Ich habe
tatsächlich mal Code gesehen, wo ein Dictionary zur
Datenübergabe verwendet wurde und die Abwesenheit eines Keys
eine andere Bedeutung hatte als der Key mit dem Wert `None`.
Das fand ich sehr verwirrend.)

Ich würde die Kommandozeilen-Optionen von den
`config`-Attributen getrennt halten, um nicht die Konzepte
Kommandozeilen-Parsen und Konfiguration zu vermischen. Das
ist aber vielleicht ein Detail.

Ich empfehle, falls möglich die `config`-Klasse erst mit
allen gewünschten Werten zu füllen oder zu überschreiben
und dann nur noch lesend darauf zuzugreifen.

Zu den Debug-Ausgaben: Da kannst du den Code etwas
vereinfachen, indem du die Abfrage des Debug-Flags in eine
Funktion verschiebst, die du aufrufst. Also statt

```
if config.debug:
print(...)

if config.debug:
print(...)

if config.debug:
print(...)
```

eher

```
def debug_print(*args, **kwargs):
if config.debug:
print(*args, flush=True, **kwargs)

debug_print(...)

debug_print(...)

debug_print(...)
```

(Bei der Gelegenheit kannst du noch wie oben ein
`flush=True` spendieren, was beim Debuggen von nebenläufigem
Code für eine übersichtlichere Ausgabe sorgen kann.)

Für `element` würde ich statt Dictionarys `namedtuple`s oder
Instanzen von `SimpleNamespace` verwenden. Allgemein sehen
Zugriffe auf Dictionarys aus meiner Sicht immer recht
"noisy" aus.

Es gibt vermutlich noch mehr zu verbessern, aber das kann
ich jetzt nicht so leicht überblicken. Oft geht solches
Aufräumen iterativ besser, also bewerten, verbessern, neu
bewerten, weiter verbessern usw.

Viele Grüße
Stefan

[Stefan Schwarzer]

On 2022-11-28 16:37, c.b...@posteo.jp wrote:
> Die Frage ist, warum du solche PyLint Ausnahmen ("# pylint:") definiert
> hast. IMHO benötigt man dafür einen wirklich triftigen Grund; eine
> strenge Indikation. Die Hinweise und Meldungen von Lintern haben ihre
> Berechtigung und sollten nicht unterdrück werden.

Ich sehe das nicht ganz so streng. Linter-Ausgaben sind für
mich eher Hinweise/Empfehlungen; letztlich sollte der/die
AutorIn des Codes entscheiden, was davon wie umgesetzt wird.

> Neben den Lintern würde ich dir noch empfehlen, ein paar mehr Leerzeilen
> zu machen. Das erhöht IMHO die Lesbarkeit, gerade bei verschachtelten
> Strukturen. Natürlich ist das Geschmacksache bzw. eine Frage des
> Team-Konsent. Z.B. hier:
>
> Am 28.11.2022 16:19 schrieb Marc Haber:
>
>> while True:
>> time.sleep(30)
>> if debug > 0:
>> print('run evaluate() and cleanup_table() from main loop')
>> cleanup_table()
>> if debug > 0:
>> print_table()
>> evaluate()
>
> while True:
>
> time.sleep(30)
>
> if debug > 0:
> print('run evaluate() and cleanup_table() from main loop')
>
> cleanup_table()
>
> if debug > 0:
> print_table()
>
> evaluate()

Sinnigerweise sehe ich das umgekehrt, das heißt, ich finde,
was die Verwendung von Leerzeilen angeht, die erste Variante
lesbarer. :-)

Oder spezieller kann man sagen, dass zwar die Lesbarkeit
_lokal_ geringfügig verbessert wird, aber durch vermehrtes
Scrollen im Code die Lesbarkeit _global_ eher schlechter
wird.

Die "großzügige" Einrückung von vier Leerzeichen pro
Einrückungs-Ebene in Python macht es eigentlich relativ
leicht, logische Strukturen zu erkennen, auch ohne die
Leerzeilen.

Zum Thema Leerzeilen innerhalb von Funktionen und Methoden
sollte man sich als Autor ruhig fragen, was man damit
bezweckt. Ich habe schon manches Mal Code in einer Methode
gesehen, der mit Leerzeilen in Blöcke aufgeteilt wurde, -
aber ich hatte keine Ahnung, warum die Blöcke so aufgeteilt
waren wie sie waren und fand das dadurch eher verwirrend.

Noch ein Tipp, den ich selbst zu beherzigen versuche: Immer,
wenn ich Leerzeilen zur Gruppierung innerhalb von Funktionen
einfügen würde, schreibe ich stattdessen einen Code-Kommentar,
der den Code-Block zusammenfasst. Man kann natürlich
trotzdem oberhalb des Zusammenfassungs-Kommentars noch eine
Leerzeile einfügen, aber ich mache es normalerweise nicht.

Zum Thema Kommentare siehe auch
https://sschwarzer.com/download/comments_pycon_de2019.pdf :-)

Viele Grüße
Stefan

[Stefan Schwarzer]

On 2022-11-28 22:07, Marc Haber wrote:
> Kann ich die Locks innerhalb der entsprechenden Funktion definieren
> oder bekomme ich dann in jedem Thread ein eigenes Lock, was der
> Intention entgegen spricht?
>
> Ist:
>
> foolock = threading.Lock()
> def foo:
> with foolock:
> (tue Dinge mit foo, potenziell multithreaded)
>
> dasselbe wie
>
> def foo:
> foolock = threading.Lock()
> with foolock:
> (tue Dinge mit foo, potenziell multithreaded)
>
> ?

Die Ansätze sind _nicht_ gleichwertig. Wie du richtig
erkannt/vermutet hast, definierst du im zweiten Ansatz bei
Ausführung von `foo` "on the fly" ein Lock-Objekt.
Dieses hat mit anderen Lock-Objekten, die in anderen
Threads in `foo` erzeugt werden, nichts zu tun. Das Lock
ist damit wirkungslos (vielleicht mal abgesehen von dem
Sonderfall, dass du das Lock in anderen Aufrufen im
`with`-Block weiterreichst, aber das ist eine ganz andere
Semantik, die du hier vermutlich nicht im Sinn hast :-) ).

> Ist die in
> https://www.bogotobogo.com/python/Multithread/python_multithreading_Using_Locks_with_statement_Context_Manager.php
> und
> https://www.pythontutorial.net/python-concurrency/python-threading-lock/
> verwendete Schreibweise, wo das Lock als Parameter mit in die Funktion
> hineingereicht wird, wirklich "schönes" Python?

Allgemein würde ich nicht `acquire` und `release` explizit
verwenden beziehungsweise die meisten der Situationen, wo
man `acquire` und `release` verwendet, können mit einem
`with some_lock:` übersichtlicher geschrieben werden.

> Oder schreibe ich mein ganzes Programm als eine Klasse, die genau
> einmal instanziiert wird und dann in ihren statischen Variablen (so
> wäre es in C++) die Tabelle, die Konfiguration, die Locks, den
> Debugstatus etc hält? Sind solche Ein-Instanz-Klassen "schönes"
> Python?

Ich kenne so etwas unter dem Namen "Active Object"-Pattern;
jedenfalls nehme ich an, dass das gemeint ist (auch wenn es
sich aus meiner Sicht etwas wüst anhört, ganz verschiedene
Sachen in die Instanz zu packen, falls das so gemeint ist).

Ein weiterer Ansatz ist, Queues zur Synchronisation zu
verwenden, was tendenziell deutlich weniger fehleranfällig
ist. Ich habe vor ein paar Jahren einen Vortrag zu
Nebenläufigkeit in Python gehalten, der auch Queues und
Active Objects behandelt:

https://sschwarzer.com/download/concurrency_pycon_de2018.pdf

Viele Grüße
Stefan

[Marc Haber]

On Tue, Nov 29, 2022 at 06:51:17PM +0100, Stefan Schwarzer wrote:
> On 2022-11-28 22:07, Marc Haber wrote:
> > Kann ich die Locks innerhalb der entsprechenden Funktion definieren
> > oder bekomme ich dann in jedem Thread ein eigenes Lock, was der
> > Intention entgegen spricht?
> >
> > Ist:
> >
> > foolock = threading.Lock()
> > def foo:
> > with foolock:
> > (tue Dinge mit foo, potenziell multithreaded)
> >
> > dasselbe wie
> >
> > def foo:
> > foolock = threading.Lock()
> > with foolock:
> > (tue Dinge mit foo, potenziell multithreaded)
> >
> > ?
>
> Die Ansätze sind _nicht_ gleichwertig. Wie du richtig
> erkannt/vermutet hast, definierst du im zweiten Ansatz bei
> Ausführung von `foo` "on the fly" ein Lock-Objekt.

Dann muss ich hier mit globalen Locks arbeiten?

> > Ist die in
> > https://www.bogotobogo.com/python/Multithread/python_multithreading_Using_Locks_with_statement_Context_Manager.php
> > und
> > https://www.pythontutorial.net/python-concurrency/python-threading-lock/
> > verwendete Schreibweise, wo das Lock als Parameter mit in die Funktion
> > hineingereicht wird, wirklich "schönes" Python?
>
> Allgemein würde ich nicht `acquire` und `release` explizit
> verwenden beziehungsweise die meisten der Situationen, wo
> man `acquire` und `release` verwendet, können mit einem
> `with some_lock:` übersichtlicher geschrieben werden.

Ja, das ist mir auch sympathischer.

> Ein weiterer Ansatz ist, Queues zur Synchronisation zu
> verwenden, was tendenziell deutlich weniger fehleranfällig
> ist.

Das könnte für die vorliegende Verwendung overkill sein.

Grüße
Marc

--
-----------------------------------------------------------------------------
Marc Haber | "I don't trust Computers. They | Mailadresse im Header
Leimen, Germany | lose things." Winona Ryder | Fon: *49 6224 1600402
Nordisch by Nature | How to make an American Quilt | Fax: *49 6224 1600421

[Stefan Schwarzer]

On 2022-11-29 21:44, Marc Haber wrote:
> On Tue, Nov 29, 2022 at 06:51:17PM +0100, Stefan Schwarzer wrote:
>> On 2022-11-28 22:07, Marc Haber wrote:
>>> Kann ich die Locks innerhalb der entsprechenden Funktion definieren
>>> oder bekomme ich dann in jedem Thread ein eigenes Lock, was der
>>> Intention entgegen spricht?
>>>
>>> Ist:
>>>
>>> foolock = threading.Lock()
>>> def foo:
>>> with foolock:
>>> (tue Dinge mit foo, potenziell multithreaded)
>>>
>>> dasselbe wie
>>>
>>> def foo:
>>> foolock = threading.Lock()
>>> with foolock:
>>> (tue Dinge mit foo, potenziell multithreaded)
>>>
>>> ?
>>
>> Die Ansätze sind _nicht_ gleichwertig. Wie du richtig
>> erkannt/vermutet hast, definierst du im zweiten Ansatz bei
>> Ausführung von `foo` "on the fly" ein Lock-Objekt.
>
> Dann muss ich hier mit globalen Locks arbeiten?

Genau, bzw. du musst sicherstellen, dass der Code im `with`-Block
nur einmal gleichzeitig ausgeführt wird. Ob das Lock global auf
Modul-Ebene oder in einem anderen Objekt gespeichert ist, ist
nicht relevant.

>> Ein weiterer Ansatz ist, Queues zur Synchronisation zu
>> verwenden, was tendenziell deutlich weniger fehleranfällig
>> ist.
>
> Das könnte für die vorliegende Verwendung overkill sein.

Ich finde die Verwendung von Queues im Allgemeinen ziemlich
intuitiv, und man kann leichter darüber nachdenken und sicher(er)
sein, dass der Code funktioniert.

Aber nimm ruhig den Ansatz, mit dem du dich besser fühlst (wenn
der Schaden im Fehlerfall nur gering ist; aber das musst du
selbst einschätzen :-) ).

Viel Spaß und viele Grüße
Stefan

[Matthias Urlichs]

On 28.11.22 21:52, Marc Haber wrote:
> Die
> Alternative, jeder Funktion das Konfig-Dictionary als Parameter
> einzufüllen, finde ich noch schlechter lesbar. Wie ist in Python das
> normale Idiom für sowas?

Du verpackst deine Funktionen in ein Objekt und gibst __init__ die
Konfig als Parameter.

Ich empfinde das nicht als Mehrarbeit, denn es hilft beim
Code-Strukturieren, mithin beim strukturiert denken, mithin beim
Fehlervermeiden.

Und wenn man nächstes Jahr zwei dieser Objekte gleichzeitig braucht,
freut man sich, dass es sie schon gibt. :-)


Globals wie "debug" oder halt "DEBUG" gehören sich nicht, finde ich;
dazu gibt es das "logging"-Modul, dem man (wenn's notwendig wird) eh
viel flexibler sagen kann, was es loggen soll, als mit einer Konstanten.
Außerdem ist "logging" weniger programmierfehleranfällig (wer hat noch
nicht versehentlich nicht-debug-Code unter ein "if debug:" gestellt …)
und visuell übersichtlicher. Und weniger tippen muss man auch …


Im Übrigen würde ich, wenn du mehr als einen Job gleichzeitig machen
willst, dringend empfehlen, das "threading"-Modul in die Tonne zu treten
und dir stattdessen "anyio" genauer anzusehen.

--
Matthias Urlichs
Executive Principal Solution Architect (Linux)

noris network AG
Thomas-Mann-Straße 16-20
90471 Nürnberg
Deutschland

Tel +49 911 9352 1717
Fax +49 911 9352 100
Email matthias...@noris.de

noris network AG - Mehr Leistung als Standard
Vorstand: Ingo Kraupa (Vorsitzender), Joachim Astel, Florian Sippel
Vorsitzender des Aufsichtsrats: Stefan Schnabel - AG Nürnberg HRB 17689

[Hans-Peter Jansen]

Am Dienstag, 29. November 2022, 18:18:38 CET schrieb Stefan Schwarzer:
> Hallo Marc,
>
> On 2022-11-28 16:19, Marc Haber wrote:
>
> > ich wollte mich schon seit Jahren mal etwas ernsthafter mit python
> > beschäftigen und hier ist mein Erstlingswerk signifikanter Länge.
> > pylint hat nichts auszusetzen, an manchen Stellen habe ich pylint aber
> > überstimmt.
> >
> > Ist da irgendetwas drin, was nicht pythonesk genug formuliert ist?
> >
> > Laufen tut das Programm jedenfalls.
> >
> > Ich würde mich über Eure Kommentare freuen.
>
>
> ich habe hier und da ein paar Anregungen. Wie immer gilt,
> dass letztlich der Autor entscheidet, wie er/sie diese
> Anregungen umsetzt. :-)

+1

> Erst mal zum Thema der Konfigurations-Speicherung, was in
> dem Diskussions-Thread auch aufgegriffen wurde. Da in Python
> Module und Klassen natürliche Singletons sind, wäre mein
> Vorschlag, die Konfigurations-Daten in einer Klasse zu
> speichern, also
>
> ```
> class config:
>
> # Listener IP address and port of MQTT server.
> ip_address = None
> port = None
>
> # If `True`, print debugging output.
> debug = False
>
> ...
> ```
>
> Du kannst einfach von überall aus dem Modul auf das
> `config`-Objekt zugreifen und brauchst auch kein `global`.
> Die Klasse wird nie instanziiert, sondern nur als Namespace
> benutzt.

Yep, dem kann ich mich nur anschließen, und ich praktiziere diese Methode seit
Jahrzehnten. Bei mir heißt diese Klasse <gpar>, da es mit <config> häufig zu
Überschneidungen von anderen Funktionalitäten kommt (z.B. config files).

Grundsätzlich gilt es dabei abzuwägen, ob Du diese Werte
* direkt wie globals verwendest <gpar.value>
-> für skriptnahe Routinen
* deinen Funktionen/Methoden gpar als Parameter <func(gpar)> übergibst
-> skriptnahe Funktionen/Instanzen, die viele der Werte brauchen
* die Konfigurationswerte einzeln übergibst <func(gpar.v1, gpar.v2)>
-> für universell wiederverwendbare Funktionen/Klassen/Methoden

Vorab, ich erhebe nicht den Anspruch, irgendwie ausgesprochen pythonistisch zu
sein, und setze in manchen Bereichen ganz bewusst oldschool-Ansätze ein (z.B.
getopt). Komme von einem systemnahen Assembler und C-Umfeld, und habe mit
Python im letzten Jahrtausend angefangen. Seitdem nutze ich es überall, wo es
die Wahl gibt, als überaus nützliches, universelles Werkzeug, dass immer noch
Spaß macht! Letzteres ist eines der wichtigsten Aspekte für die Beliebtheit
der Sprache, die nicht in irgendwelchen strategischen Charts ihren Einstieg in
Industrie-Projekte gefunden hat (wie beispielsweise Java).

Zu dem schon Gesagten möchte ich noch ergänzen:

Aus Gründen der Flexibilität verwende ich für einfache Skripte folgendes
Muster:

def main(argv = None):
"""Command line interface and console script entry point."""
if argv is None:
argv = sys.argv[1:]
[...]
return 0

if __name__ == '__main__':
sys.exit(main())

So kannst Du den Funktionsumfang des Skripts direkt, von anderen Modulen und
auch mit setuptools benutzen.

Die --help Ausgabe findet sich in meinen Skripten meistens im __doc__ Bereich
des Hauptmoduls, wie Du auch schon damit angefangen hast. So habe ich auch die
Dokumentation da, wo ich sie *will*. Neben vielen Vorteilen hat das aber auch
den Nachteil, Kommandozeilen Optionen nicht an einer Stelle fokussiert
abzufrühstücken. Das funktioniert gut bis zu einer gewissen Größe des
Projekts.

Wie das in echt aussieht, kannst Du Dir ja mal hier anschauen:

https://github.com/frispete/vpndnshelper/blob/main/vpndnshelper.py

Vielleicht findest Du da auch noch die eine oder andere Anregung. Da Du dich
scheinbar auch in einem unixoiden Umfeld bewegst, könnten dich die signal und
exception Behandlungen der main Funktion interessieren. Dieses Skript läuft
übrigens üblicherweise als systemd Dienst, hat aber noch nie einen Linter
gesehen (und ja, das sollte ich auch mal tun..).

Cheers,
Pete
--
Life without chameleons is possible, but pointless.

[Marc Haber]

On Tue, Nov 29, 2022 at 06:18:38PM +0100, Stefan Schwarzer wrote:
> Erst mal zum Thema der Konfigurations-Speicherung, was in
> dem Diskussions-Thread auch aufgegriffen wurde. Da in Python
> Module und Klassen natürliche Singletons sind, wäre mein
> Vorschlag, die Konfigurations-Daten in einer Klasse zu
> speichern, also
>
> ```
> class config:
>
> # Listener IP address and port of MQTT server.
> ip_address = None
> port = None
>
> # If `True`, print debugging output.
> debug = False
>
> ...
> ```
>
> Du kannst einfach von überall aus dem Modul auf das
> `config`-Objekt zugreifen und brauchst auch kein `global`.
> Die Klasse wird nie instanziiert, sondern nur als Namespace
> benutzt.

Habe ich ausprobiert, funktioniert. Nur pylint meckert:
too-few-public-methods: Too few public methods (0/2)

Ist das sowas ähnliches wie eine @dataclass?

Und eigentlich ist das doch auch nur ein Satz globaler Variablen auf
Steroiden.

> Der Ansatz hat gegenüber dem `config`-Dictionary auch den
> Vorteil, dass du dokumentieren kannst, wofür die einzelnen
> Werte gedacht sind. Ich halte es für gut, in `config` alle
> Werte auf Default-Werte zu setzen, selbst wenn diese mangels
> eines sinnvollen Defaults erst mal `None` sind. Das hat den
> Vorteil, dass man sehen kann, was jemals gesetzt wird und
> reduziert die Verwirrung, weil man nicht zwischen `None` und
> "gar nicht gesetzt" unterscheiden muss.

Andererseits muss ich halt jede auftretende Konfigurationsvariable an
mindestens zwei zusätzlichen Stellen explizit hinschreiben, während sich
das bei einem confighash direkt selbsttätig ergibt.

> Ich würde die Kommandozeilen-Optionen von den
> `config`-Attributen getrennt halten, um nicht die Konzepte
> Kommandozeilen-Parsen und Konfiguration zu vermischen. Das
> ist aber vielleicht ein Detail.

Ich finde es eigentlich ganz gut, wenn es für Konfigurationsvariablen
auch eine entsprechende Kommandozeilenoption gibt. Auf diese Weise kann
man die regelmäßig verwendeten Optionen in die Konfigurationsdatei
schreiben und wenn man es doch mal anders braucht auf der Kommandozeile
überschreiben.

> Zu den Debug-Ausgaben: Da kannst du den Code etwas
> vereinfachen, indem du die Abfrage des Debug-Flags in eine
> Funktion verschiebst, die du aufrufst.

Das habe ich jetzt mit dem Logging-Modul gemacht. Der Nachteil ist nur
dass ich jetzt meiner Tabellenklasse selbst beibringen muss, sich
auszudrucken, weil ich sonst durch die kompletten Bewegungen des
Ausdrucks gehe und im Zweifel nur die eigentliche Ausgabe unterdrücke.

> Für `element` würde ich statt Dictionarys `namedtuple`s oder
> Instanzen von `SimpleNamespace` verwenden. Allgemein sehen
> Zugriffe auf Dictionarys aus meiner Sicht immer recht
> "noisy" aus.

Das lasse ich erstmal, habe mir das notiert.

[Marc Haber]

On Wed, Nov 30, 2022 at 10:47:53AM +0000, Matthias Urlichs wrote:
> On 28.11.22 21:52, Marc Haber wrote:
> > Die
> > Alternative, jeder Funktion das Konfig-Dictionary als Parameter
> > einzufüllen, finde ich noch schlechter lesbar. Wie ist in Python das
> > normale Idiom für sowas?
>
> Du verpackst deine Funktionen in ein Objekt und gibst __init__ die
> Konfig als Parameter.

Auch wieder so eine Klasse die man niemals instanziiert? Dann sind die
"globals" in der Klasse gekapselt und nicht im Modul, das ist doch nur
eine Begrifflichkeit.

> Und wenn man nächstes Jahr zwei dieser Objekte gleichzeitig braucht,
> freut man sich, dass es sie schon gibt. :-)

Dann muss man aber doch die Klasse anders schreiben?

> Globals wie "debug" oder halt "DEBUG" gehören sich nicht, finde ich;
> dazu gibt es das "logging"-Modul, dem man (wenn's notwendig wird) eh
> viel flexibler sagen kann, was es loggen soll, als mit einer Konstanten.
> Außerdem ist "logging" weniger programmierfehleranfällig (wer hat noch
> nicht versehentlich nicht-debug-Code unter ein "if debug:" gestellt …)
> und visuell übersichtlicher. Und weniger tippen muss man auch …

Weniger tippen kann ich jetzt nicht sagen, ich hab hier jetzt:
logger = logging.getLogger(__name__)↲
loggingStreamHandler = logging.StreamHandler()↲
logger.addHandler(loggingStreamHandler)↲
und weiter unten (da wo die Kommandozeilenparameter gelesen werden):
if args.debug:
loggingStreamHandler.setLevel(10)↲
logger.setLevel(10)↲
und noch weiter unten (da wo die Konfigurationsdatei gelesen wird):
if not args.debug:
loggingStreamHandler.setLevel(config.debuglevel)↲
logger.setLevel(config.debuglevel)↲

Bin mir noch nicht sicher was da die richtige Bedingung sein muss,
eigentlich möchte ich dass args.debug ein config.debug überschreibt.

> Im Übrigen würde ich, wenn du mehr als einen Job gleichzeitig machen
> willst, dringend empfehlen, das "threading"-Modul in die Tonne zu treten
> und dir stattdessen "anyio" genauer anzusehen.

Das kann ich mir wohl nicht aussuchen, weil ich hier mit dem paho-Modul
für MQTT koexistieren muss.

[Stefan Schwarzer]

On 2022-12-01 21:16, Marc Haber wrote:
> On Tue, Nov 29, 2022 at 06:18:38PM +0100, Stefan Schwarzer wrote:
>> Du kannst einfach von überall aus dem Modul auf das
>> `config`-Objekt zugreifen und brauchst auch kein `global`.
>> Die Klasse wird nie instanziiert, sondern nur als Namespace
>> benutzt.
>
> Habe ich ausprobiert, funktioniert. Nur pylint meckert:
> too-few-public-methods: Too few public methods (0/2)

Hihi, ja, das ist wieder das "Problem" mit Pylint. ;-)

> Ist das sowas ähnliches wie eine @dataclass?

`dataclass` ist ähnlich. Ich hatte auch daran gedacht, aber
wollte es vermeiden, in die Diskussion abdriften, ob/wo man
in Python explizite Typ-Angaben verwenden sollte. Daher habe
ich den "normaleren" Ansatz vorgeschlagen.

> Und eigentlich ist das doch auch nur ein Satz globaler Variablen auf
> Steroiden.

Ja, das kann man so sehen. Ich finde die Extra-Klasse ganz
schön, um zusammengehörige Objekte zu gruppieren. Ich würde
das hier wahrscheinlich machen, aber man kann auch
argumentieren, dass es bei drei(?) Objekten in der Klasse
auch ok ist, Variablen auf der Modul-Ebene zu verwenden.
Letztlich ist das Geschmackssache, denke ich.

>> Der Ansatz hat gegenüber dem `config`-Dictionary auch den
>> Vorteil, dass du dokumentieren kannst, wofür die einzelnen
>> Werte gedacht sind. Ich halte es für gut, in `config` alle
>> Werte auf Default-Werte zu setzen, selbst wenn diese mangels
>> eines sinnvollen Defaults erst mal `None` sind. Das hat den
>> Vorteil, dass man sehen kann, was jemals gesetzt wird und
>> reduziert die Verwirrung, weil man nicht zwischen `None` und
>> "gar nicht gesetzt" unterscheiden muss.
>
> Andererseits muss ich halt jede auftretende Konfigurationsvariable an
> mindestens zwei zusätzlichen Stellen explizit hinschreiben, während sich
> das bei einem confighash direkt selbsttätig ergibt.

Yep, ist wieder eine Abwägung, also die Frage, welche
Kombination von Vor- und Nachteilen man bevorzugt.

>> Ich würde die Kommandozeilen-Optionen von den
>> `config`-Attributen getrennt halten, um nicht die Konzepte
>> Kommandozeilen-Parsen und Konfiguration zu vermischen. Das
>> ist aber vielleicht ein Detail.
>
> Ich finde es eigentlich ganz gut, wenn es für Konfigurationsvariablen
> auch eine entsprechende Kommandozeilenoption gibt. Auf diese Weise kann
> man die regelmäßig verwendeten Optionen in die Konfigurationsdatei
> schreiben und wenn man es doch mal anders braucht auf der Kommandozeile
> überschreiben.

Wenn du eine starke Überschneidung zwischen der
Konfiguration im allgemeinen und der Konfiguration von der
Kommandozeile hast, ist das plausibel, nur eine
Klasse/Namespace zu verwenden. Es gibt aber auch Fälle, wo
es keine so starke Überschneidung gibt, zum Beispiel, weil
Konfigurations-Einstellungen aus verschiedenen Quellen
kommen (im Code hartverdrahtet, von der Kommandozeile, aus
Konfigurationsdateien, etc.). Wenn das viel Konfiguration
ist, muss/sollte man das aber wiederum gegebenenfalls anders
angehen. Du entscheidest. :-)

>> Zu den Debug-Ausgaben: Da kannst du den Code etwas
>> vereinfachen, indem du die Abfrage des Debug-Flags in eine
>> Funktion verschiebst, die du aufrufst.
>
> Das habe ich jetzt mit dem Logging-Modul gemacht. Der Nachteil ist nur
> dass ich jetzt meiner Tabellenklasse selbst beibringen muss, sich
> auszudrucken, weil ich sonst durch die kompletten Bewegungen des
> Ausdrucks gehe und im Zweifel nur die eigentliche Ausgabe unterdrücke.

Ich finde das gut, dass du mit verschiedenen Ansätzen
experimentierst. Dann bekommst du ein noch besseres Gefühl
dafür, welche Vor- und Nachteile verschiedene Ansätze haben.
:-)

Viele Grüße
Stefan

[Stefan Schwarzer]

On 2022-12-01 21:30, Marc Haber wrote:
> On Wed, Nov 30, 2022 at 10:47:53AM +0000, Matthias Urlichs wrote:
>> On 28.11.22 21:52, Marc Haber wrote:
>>> Die
>>> Alternative, jeder Funktion das Konfig-Dictionary als Parameter
>>> einzufüllen, finde ich noch schlechter lesbar. Wie ist in Python das
>>> normale Idiom für sowas?
>>
>> Du verpackst deine Funktionen in ein Objekt und gibst __init__ die
>> Konfig als Parameter.
>
> Auch wieder so eine Klasse die man niemals instanziiert? Dann sind die
> "globals" in der Klasse gekapselt und nicht im Modul, das ist doch nur
> eine Begrifflichkeit.

Ich denke auch, das ist sehr ähnlich.

>> Globals wie "debug" oder halt "DEBUG" gehören sich nicht, finde ich;
>> dazu gibt es das "logging"-Modul, dem man (wenn's notwendig wird) eh
>> viel flexibler sagen kann, was es loggen soll, als mit einer Konstanten.
>> Außerdem ist "logging" weniger programmierfehleranfällig (wer hat noch
>> nicht versehentlich nicht-debug-Code unter ein "if debug:" gestellt …)
>> und visuell übersichtlicher. Und weniger tippen muss man auch …
>
> Weniger tippen kann ich jetzt nicht sagen, ich hab hier jetzt:
> logger = logging.getLogger(__name__)↲
> loggingStreamHandler = logging.StreamHandler()↲
> logger.addHandler(loggingStreamHandler)↲
> und weiter unten (da wo die Kommandozeilenparameter gelesen werden):
> if args.debug:
> loggingStreamHandler.setLevel(10)↲
> logger.setLevel(10)↲
> und noch weiter unten (da wo die Konfigurationsdatei gelesen wird):
> if not args.debug:
> loggingStreamHandler.setLevel(config.debuglevel)↲
> logger.setLevel(config.debuglevel)↲

In manchen Fällen kommt man auch mit dem Aufruf von
`logging.basicConfig` aus. Ich habe mir deinen Code aber
nicht genau angesehen, inwieweit das für dich anwendbar ist.

>> Im Übrigen würde ich, wenn du mehr als einen Job gleichzeitig machen
>> willst, dringend empfehlen, das "threading"-Modul in die Tonne zu treten
>> und dir stattdessen "anyio" genauer anzusehen.
>
> Das kann ich mir wohl nicht aussuchen, weil ich hier mit dem paho-Modul
> für MQTT koexistieren muss.

Ich kannte `anyio` noch nicht und habe jetzt nur das
Inhaltsverzeichnis überflogen. Das ist nach meinem Eindruck
schon eine ganze Menge Komplexitäts- und damit Lern-Aufwand,
und dann stellt sich immer die Frage, ob es sich lohnt, so
eine komplexe Abhängigkeit zu nutzen, wenn man vielleicht
nur einen ganz kleinen Anteil davon braucht.

Mit dem Logging-Modul ist das ähnlich, aber da ist es
immerhin so, dass es so bekannt und "universell" ist, dass
viele damit sowieso schon gearbeitet haben.

Viele Grüße
Stefan

[Matthias Urlichs]

On 01.12.22 21:30, Marc Haber wrote:
> On Wed, Nov 30, 2022 at 10:47:53AM +0000, Matthias Urlichs wrote:
>> On 28.11.22 21:52, Marc Haber wrote:
>>> Die
>>> Alternative, jeder Funktion das Konfig-Dictionary als Parameter
>>> einzufüllen, finde ich noch schlechter lesbar. Wie ist in Python das
>>> normale Idiom für sowas?
>> Du verpackst deine Funktionen in ein Objekt und gibst __init__ die
>> Konfig als Parameter.
> Auch wieder so eine Klasse die man niemals instanziiert?

Wieso sollte man die nicht instanziieren?

>> dazu gibt es das "logging"-Modul, dem man (wenn's notwendig wird) eh
>> viel flexibler sagen kann, was es loggen soll, als mit einer Konstanten.
>> Außerdem ist "logging" weniger programmierfehleranfällig (wer hat noch
>> nicht versehentlich nicht-debug-Code unter ein "if debug:" gestellt …)
>> und visuell übersichtlicher. Und weniger tippen muss man auch …
> Weniger tippen kann ich jetzt nicht sagen, ich hab hier jetzt:
> logger = logging.getLogger(__name__)
> loggingStreamHandler = logging.StreamHandler()
> logger.addHandler(loggingStreamHandler)

Du hast

logging.basicConfig(level=logging.DEBUG if args.debug else
logging.WARNING)
log = logging.getLogger("main")

übersehen.

Außerdem meinte ich eher die eigentlichen Logging-Aufrufe. Da kannst du
dir die if-Abfrage plus Einrückung sparen und einfach

log.debug("Test %d", 42)

schreiben.

>
> Das kann ich mir wohl nicht aussuchen, weil ich hier mit dem paho-Modul
> für MQTT koexistieren muss.

Musst du das wirklich? Mein "moat-mqtt"-Modul kann das unter anyio (und
da ist auch ein Broker dabei, den man im selben Programm mitlaufen
lassen kann).

"trio_paho_mqtt" funktioniert auch, wenn du anyio sagst dass es das
Trio-Backend verwenden soll.

[Marc Haber]

Hallo,

On Fri, Dec 02, 2022 at 09:29:58AM +0000, Matthias Urlichs wrote:
> On 01.12.22 21:30, Marc Haber wrote:
> > On Wed, Nov 30, 2022 at 10:47:53AM +0000, Matthias Urlichs wrote:
> >> On 28.11.22 21:52, Marc Haber wrote:
> >>> Die
> >>> Alternative, jeder Funktion das Konfig-Dictionary als Parameter
> >>> einzufüllen, finde ich noch schlechter lesbar. Wie ist in Python das
> >>> normale Idiom für sowas?
> >> Du verpackst deine Funktionen in ein Objekt und gibst __init__ die
> >> Konfig als Parameter.
> > Auch wieder so eine Klasse die man niemals instanziiert?
>
> Wieso sollte man die nicht instanziieren?

Ich hatte Dein Konstrukt nicht richtig verstanden.

> >> dazu gibt es das "logging"-Modul, dem man (wenn's notwendig wird) eh
> >> viel flexibler sagen kann, was es loggen soll, als mit einer Konstanten.
> >> Außerdem ist "logging" weniger programmierfehleranfällig (wer hat noch
> >> nicht versehentlich nicht-debug-Code unter ein "if debug:" gestellt …)
> >> und visuell übersichtlicher. Und weniger tippen muss man auch …
> > Weniger tippen kann ich jetzt nicht sagen, ich hab hier jetzt:
> > logger = logging.getLogger(__name__)
> > loggingStreamHandler = logging.StreamHandler()
> > logger.addHandler(loggingStreamHandler)
>
> Du hast
>
> logging.basicConfig(level=logging.DEBUG if args.debug else
> logging.WARNING)
> log = logging.getLogger("main")
>
> übersehen.

basicConfig möchte ich nicht verwenden, weil damit mein bevorzugtes
Konstrukt, den Debuglevel sowohl aus dem Konfigurationsfile als auch aus
der Kommandozeile zu setzen UND über das Lesen des Konfiguationsfiles
und dem Parsen der Kommandozeile bereits sinnvoll loggen zu können nicht
geht. bsaicConfig kann man nur einmal benutzen, dann sind die
Einstellungen für den Rest der Lebenszeit des Loggers fest.

> Außerdem meinte ich eher die eigentlichen Logging-Aufrufe. Da kannst du
> dir die if-Abfrage plus Einrückung sparen und einfach
>
> log.debug("Test %d", 42)
>
> schreiben.

Ja, dafür zahlt man halt an anderer Stelle einen Preis.

> > Das kann ich mir wohl nicht aussuchen, weil ich hier mit dem paho-Modul
> > für MQTT koexistieren muss.
>
> Musst du das wirklich?

Nein, muss ich freilich nicht. Aber ich möchte mich jetzt nicht
verkünsteln. Die Aufgabe, die das Programm erledigen soll, ist einfach,
das Programm dient mir nebenbei auch noch dazu, ein Gefühl für die mir
neue Programmiersprache zu entwickeln. Und dieses Gefühl bekomme ich
nicht, wenn ich gleich alle Lösungen verwende die für großindustrielle
Umgebungen skalieren müssen.

> "trio_paho_mqtt" funktioniert auch, wenn du anyio sagst dass es das
> Trio-Backend verwenden soll.

Dann müsste ich zusätzlich noch trio lernen. Das ist mir aktuell etwas
viel, ich möchte das Programm noch dieses Jahr fertig haben, sonst lacht
mich meine Frau aus.

[Marc Haber]

On Thu, Dec 01, 2022 at 11:58:31PM +0100, Stefan Schwarzer wrote:
> On 2022-12-01 21:16, Marc Haber wrote:
> > On Tue, Nov 29, 2022 at 06:18:38PM +0100, Stefan Schwarzer wrote:
> > > Du kannst einfach von überall aus dem Modul auf das
> > > `config`-Objekt zugreifen und brauchst auch kein `global`.
> > > Die Klasse wird nie instanziiert, sondern nur als Namespace
> > > benutzt.
> >
> > Habe ich ausprobiert, funktioniert. Nur pylint meckert:
> > too-few-public-methods: Too few public methods (0/2)
>
> Hihi, ja, das ist wieder das "Problem" mit Pylint. ;-)

Das ist so eine Warning, die ich vermutlich irgendwann global per
Konfigurationsfile ausschalten werde, genauso wie die, die
allgemein übliche Variablennamen wie "i" für einen Zähler oder "ts" für
einen Timestamp als "nicht ím snake_case geschrieben" anmeckert.

> > Und eigentlich ist das doch auch nur ein Satz globaler Variablen auf
> > Steroiden.
>
> Ja, das kann man so sehen. Ich finde die Extra-Klasse ganz
> schön, um zusammengehörige Objekte zu gruppieren. Ich würde
> das hier wahrscheinlich machen, aber man kann auch
> argumentieren, dass es bei drei(?) Objekten in der Klasse
> auch ok ist, Variablen auf der Modul-Ebene zu verwenden.
> Letztlich ist das Geschmackssache, denke ich.

Ich habe das Gefühl, dass in python ganz besonders wenig als
"Geschmackssache" durchgeht und ziemlich viel "das schreibt und
formatiert man so, basta" gehandhabt wird. Daran muss ich mich erstmal
gewöhnen, ich komme eher aus der "there is more than one way to do it"
Ecke. Aber ich versuche, ein Gefühl zu bekommen, vielleicht gefällt es
mir ja.

Ich habe sehr viel in Perl gemacht, weil es für alles und jedes ein
Modul gibt, das ist in Python (inzwischen) auch so, und einiges in ruby,
weil das wenn man im puppet-Umfeld arbeitet einfach die "natürliche"
Sprache ist.

> > Ich finde es eigentlich ganz gut, wenn es für Konfigurationsvariablen
> > auch eine entsprechende Kommandozeilenoption gibt. Auf diese Weise kann
> > man die regelmäßig verwendeten Optionen in die Konfigurationsdatei
> > schreiben und wenn man es doch mal anders braucht auf der Kommandozeile
> > überschreiben.
>
> Wenn du eine starke Überschneidung zwischen der
> Konfiguration im allgemeinen und der Konfiguration von der
> Kommandozeile hast

Das ist bei meinen kleinen praktischen Kommandozeilentools recht häufig
der Fall. Erschwerend kommt hinzu, dass ich eine schwere Abneigung gegen
ini-Files habe und yaml vorziehe.

> Ich finde das gut, dass du mit verschiedenen Ansätzen
> experimentierst. Dann bekommst du ein noch besseres Gefühl
> dafür, welche Vor- und Nachteile verschiedene Ansätze haben.

Darum geht es mir ja, ich möchte ein Gefühl für die Sprache bekommen.
Das Logging-Modul gefällt mir schonmal so gut, dass ich mich bei meiner
nächsten größeren perl-Arbeit¹ an den python-Idiomen orientieren werde.

Grüße
Marc

¹ das Logging in Debians adduser
(https://tracker.debian.org/pkg/adduser) ist grauslich, das gehört
dringend mal gerade/orthogonal gezogen, am besten noch vor dem nächsten
Freeze

[Peter J. Holzer]

On 2022-12-01 23:07, Stefan Schwarzer <sschw...@sschwarzer.net> wrote:
> On 2022-12-01 21:30, Marc Haber wrote:
>> Weniger tippen kann ich jetzt nicht sagen, ich hab hier jetzt:
>> logger = logging.getLogger(__name__)↲
>> loggingStreamHandler = logging.StreamHandler()↲
>> logger.addHandler(loggingStreamHandler)↲
>> und weiter unten (da wo die Kommandozeilenparameter gelesen werden):
>> if args.debug:
>> loggingStreamHandler.setLevel(10)↲
>> logger.setLevel(10)↲
>> und noch weiter unten (da wo die Konfigurationsdatei gelesen wird):
>> if not args.debug:
>> loggingStreamHandler.setLevel(config.debuglevel)↲
>> logger.setLevel(config.debuglevel)↲
>
> In manchen Fällen kommt man auch mit dem Aufruf von
> `logging.basicConfig` aus.

Das ist etwas sehr "basic". Aber sofern man die Konfiguration nicht zur
Laufzeit ändern will, dürfte logging.config.dictConfig fast immer
ausreichend sein. Das dict kann man bei Bedarf auch aus einem JSON oder
YAML-File einlesen.

[Peter J. Holzer]

On 2022-12-02 17:10, Marc Haber <mh+pyt...@zugschlus.de> wrote:
> Ich habe das Gefühl, dass in python ganz besonders wenig als
> "Geschmackssache" durchgeht und ziemlich viel "das schreibt und
> formatiert man so, basta" gehandhabt wird.

"There should be one-- and preferably only one --obvious way to do it."
vs.
TIMTOWTDI
?

Stimmt schon. Aber ich glaube, dass Du da die Meinung einzelner
Programmierer zu sehr mit "das wird in Python so gehandhabt"
gleichsetzt.

hp