UnboundLocalError im Closure

[snafu]

Hallo Forum!

Im Rahmen eines kleinen Hobby-Projekts habe ich mir einen Validator gebastelt, der vom Benutzer entsprechend konfiguriert werden kann:

Code: Alles auswählen

def choice(low, high, converter=None):
    def validator(value):
        if converter is None:
            converter = type(low)
        converted = converter(value)
        if not low <= converted <= high:
            raise ValueError("Invalid choice")
        return converted
    return validator

Ich denke, das Vorgehen ist klar. Die zurück gelieferte Funktion wird dann einer anderen Funktion als Parameter übergeben und von ihr an der passenden Stelle aufgerufen, um eine Benutzereingabe zu überprüfen. An sich sollte das ja funktionieren, da im Closure weiterhin der ursprüngliche Namensraum gelten müsste. Zu meiner Überraschung wirft das aber einen Fehler:

Code: Alles auswählen

<ipython-input-3-9c9420254249> in validator(value)
      1 def choice(low, high, converter=None):
      2     def validator(value):
----> 3         if converter is None:
      4             converter = type(low)
      5         converted = converter(value)

UnboundLocalError: local variable 'converter' referenced before assignment

Es klappt aber, wenn ich den Namen in die Signatur des Closures ziehe: ``def validator(value, converter=converter):``. Mit dem low und high hat er keine Probleme. Warum ist das so? Liegt es am Keyword-Argument?

[snafu]

Hab die Lösung schon. Es liegt daran, dass ich in meinem if-Block den Namen ``converter`` neu belege. Wird hier ganz gut erklärt: https://stackoverflow.com/a/29639807

[__blackjack__]

Wenn ich keinen Konverter angebe hätte ich ja erwartet das auch nichts konvertiert wird. Was ist denn wenn sich der Datentyp von `low` gar nicht dafür eignet eine Konvertierung durchzuführen?

Code: Alles auswählen

def choice(low, high, converter=lambda value: value):
    def validator(value):
        converted = converter(value)
        if not low <= converted <= high:
            raise ValueError("Invalid choice")
        return converted
    return validator

`choice()` wäre nicht meine Wahl für den Namen, denn ob das nun eine Auswahl war die der Benutzer da übergibt, hängt ja vom Kontext ab wo der Wert her kommt.

[snafu]

Wenn ich keinen Konverter angebe hätte ich ja erwartet das auch nichts konvertiert wird. Was ist denn wenn sich der Datentyp von `low` gar nicht dafür eignet eine Konvertierung durchzuführen?

Ich hatte das nicht erwähnt: Die Eingaben sind immer Strings. Es geht vorrangig darum, eine Menüauswahl zu realisieren oder Zahlen nur in einem bestimmten Bereich zu erlauben. Also sowas wie choice("a", "f") oder choice(1, 10).

Als Vorbelegung für den converter-Parameter schwanke ich gerade zwischen ``str`` und der von dir gezeigten Identitätsfunktion. Letzteres wäre mathematisch gesehen sauberer, ist aber auch etwas sperriger in der Lesbarkeit. str() verhält sich ja bei Strings im Endeffekt genau so (``str(s) is s``), wobei das wahrscheinlich ein Implementierungsdetail ist.

Ich wollte halt eigentlich vermeiden, dass man choice(1, 10, int) schreiben muss, auch wenn das automatische Ermittelns des Typs etwas magischer erscheint. Es ist andererseits auch eine Vereinfachung, weil man nicht jedes Mal daran denken müsste, am Ende die Typangabe zu machen.

Und um deine Frage zu beantworten: Wenn ein ungeeigneter Datentyp übergeben wird, dann sollte es mit einem TypeError krachen. Das wäre hier auch das von mir gewünschte Verhalten. Daher beschränke ich mich auf den ValueError bei der Ausnahmebehandlung.

[DeaD_EyE]

Alternative:

Code: Alles auswählen

def choice(low, high, converter=None):
    converter = converter or type(low)
    
    def validator(value):
        converted = converter(value)
        if not low <= converted <= high:
            raise ValueError("Invalid choice")
        return converted

    return validator

Ansonsten gäbe es noch das Keyword nonlocal, dass so ähnlich wie global ist, sich aber auf die darüberliegende Funktion bezieht.
Kennen die wenigsten und wird kaum verwendet.

[__blackjack__]

@snafu: Gerade das was Du vermeiden willst, klappt ja nicht mit allen Datentypen und es erscheint mir ein bisschen willkürlich, das der Code bei einem `low` Wert automagisch klappt dessen `__init__()` mit nur einem Argument aufgerufen werden kann, und dann auch noch mit dem Typ/Wert von `value` klar kommt, aber das nicht funktioniert wenn das zufälligerweise nicht zutrifft. Die Typen von `low` und in was konvertiert werden soll, müssen doch gar nicht auf diese Weise kompatibel sein. Einfache Fälle wären ich will ein `float` habe aber ganze Zahlen als Grenzen. Das ich dann die Wahl zwischen ``choice(1, 10, float)`` und ``choice(1.0, 10)`` habe ist mir zu schräg und zu subtil. Und wenn mir die Untergrenze egal ist, würde ich -unendlich einsetzen, müsste dann aber `int` als Konverter angeben wenn ich ganze Zahlen will. Während ich das nicht machen müsste wenn ich eine konkrete ganze Zahl als Untergrenze und +unendlich als Obergrenze angebe.

[snafu]

Habe mich unter Einbeziehung der berechtigten Einwände nun zu einem OOP-Ansatz entschieden:

Code: Alles auswählen

class Converter:
    def __init__(self, value_type):
        self.value_type = value_type

    def __call__(self, value):
        return self.convert(value)

    def convert(self, value):
        return self.value_type(value)


class Choice(Converter):
    def __init__(self, value_type, low, high, casefold=False):
        Converter.__init__(self, value_type)
        self.low = low
        self.high = high
        self.casefold = casefold

    def _get_operands(self, value):
        operands = [self.low, value, self.high]
        if self.casefold:
            operands = [op.casefold() for op in operands]
        return operands

    def convert(self, value):
        value = Converter.convert(self, value)
        low, choice, high = self._get_operands(value)
        if not low <= choice <= high:
            raise ValueError("Invalid choice")
        return choice

Ist natürlich mehr Code, aber dafür IMHO auch sauberer umgesetzt.

[Sirius3]

Die Klasse `Convert` ist ziemlich kompliziert `value_type` ausgedrückt.
Und ein Choice ist kein Converter. Die ist-ein-Beziehung durch die Vererbung ist daher falsch.
Die _get_operands-Methode ist auch sehr undurchsichtig, dadurch, dass da eine Liste erzeugt wird, und zwar aus einer Mischung aus Attributen und dem übergebenen value.
Die Casefold-Funktionalität sollte man besser in die Convertierfunktion einbauen.

Code: Alles auswählen

class Choice:
    def __init__(self, convert, low, high):
        self.convert = convert
        self.low = convert(low)
        self.high = convert(high)

    def __call__(self, value):
        value = self.convert(value)
        if not self.low <= value <= self.high:
            raise ValueError("Invalid choice")
        return value


choice = Choice(str.casefold, 'A', 'F')

[snafu]

@Sirius3
Jepp, dein Code macht die Sache deutlich einfacher.

Danke für eure Hilfe!

[__blackjack__]

Wobei wir bei einer Klasse die nur eine `__init__()` und eine `__call__()` hat, ja eigentlich schon wieder bei einer Funktion wären die zur Klasse aufgepustet wurde. Im Grunde braucht man noch nicht einmal ein Closure schreiben wenn man `functools.partial()` nimmt um die Argumente zu binden.

Code: Alles auswählen

from functools import partial


def validate(convert, low, high, value):
    value = convert(value)
    if not low <= value <= high:
        raise ValueError("Invalid choice")
    return value


choice = partial(validate, str.casefold, "A", "F")

[snafu]

Und so schließt sich der Kreis:

Code: Alles auswählen

def choice(convert, low, high):
    def validate(user_string):
        value = convert(user_string)
        if not low <= value <= high:
            raise ValueError("Invalid choice")
        return value
    return validate