Unverständlicher Fehler mit einer Liste

[Matze27396]

Hallo zusammen,

bin noch totaler Anfänger in der Pythonprogrammierung. Es klappte jedoch mit meinem kleinen Projekt recht gut. Jedoch habe ich einen komischen Fehler. Ich lese Daten einer Textdatei ein(konkret kommt es um eine Netzplandarstellung). Jedes gelesene Paket wird in ein Objekt geladen dass aus Name, Zeitaufwand, sowie eine Liste der Nachfolgerpakete geschrieben.
Am Schluss der Funktion wird das Netplan-Dictionary(tempNetplan) in das alle Pakete eingetragen sind zurückgegeben.
Nun zum Problem:
- Am Ende der Funktion sobald ich das Paket ausgebe(print) sind noch alle Daten da
- Sobald ich das dictionary aber auf das eigentliche übergebe(in der aufrufenden Funktion quasi) sind alle Daten bis auf die Liste verfügbar. Die Liste ist immer leer! Ich verstehe das nicht.


Plan laden:

Code: Alles auswählen

  def __LoadPlan(self,filepath):
        tempNetplan = dict()
        validatekeylist = list()
        try:
            lines = open(filepath, 'r')
        except:
            print('EXCEPTION: Problem while loading data. Please check path or dataformat')
            return tempNetplan.clear()
        for line in lines:
            columns = ''.join(line).split(" ")
            if (columns[0] in validatekeylist):
                print('EXCEPTION: Loop has been found!')
                return tempNetplan.clear()
            else:
                tempplannode = NetplanNode()
                tempplannode.Amount = int(columns[1])
                try:      
                    tempplannode.Followers.clear()
                    if (len(columns) > 2):
                        follower = columns[2].split(",")
                        for singlefollower in follower:
                            tempplannode.Followers.append(singlefollower.rstrip())
                except IndexError:
                    print('EXCEPTION: IndexError in line: ' + line)
                    return tempNetplan.clear()
                tempNetplan[columns[0]] = tempplannode
                validatekeylist.append(columns[0])
                #print(columns[0] + ': ' + str(self.__netplan[columns[0]].Amount) + ' ' + str(self.__netplan[columns[0]].Followers))
        return tempNetplan
 

Aufrufenden Funktion:

Code: Alles auswählen

    def __init__(self, path):
        self.__netplan = self.__LoadPlan(path)

Arbeitspaket Klasse bzw. Objekt:

Code: Alles auswählen

class NetplanNode(object):
    ## Public Classmembers
    Amount      = 0
    Followers   = list()
    FAZ = 0
    SAZ = 0
    GP = 0
    FP = 0
    FEZ = 0
    SEZ = 0
    Critical = False

[bords0]

Da ist einiges nicht so ganz pythonisch. Unter anderem:

Doppelte Unterstriche am Anfang sind nicht für "private" Attribute da, die gibt es nicht in Python. Einfach weglassen, bis du weißt, was sie bedeuten.
Namenskonventionen (PEP beachten, es sollte eher `def load_plan` heißen.
`self` ist unnötig, da nicht verwendet. Das sollte keine Methode einer Klasse sein (welcher?) sondern eine Funktion.

Wieder PEP 8: net_plan statt tempNetplan.

validatekeylist ist unnötig, da die keys ja schon im dictionary net_plan zur Verfügung stehen.

Die Ausnahmebehandlung ist m.E. nicht so günstig. Erstmal keine Ausnahmen abfangen, es sei denn, man kann was vernünfigtes damit machen. Eine print-Anweisung und Rückgabe eines unnützen Wertes ist meistens nichts vernünftiges - das könnte der Rufer der Funktion im Zweifel nämlich besser!
Deshalb die Datei mit `with open(filepath, 'r') as lines:` öffnen.
`''.join(line)` ist überflüssig, das ist das gleiche wie `line`.

`tempNetplan.clear()` löscht zwar das dict, hat aber den Rückgabewert `None`. Eine simples `return` würde also das gleiche bewirken. Aber wie gesagt ist es besser, die Ausnahme nicht explizit zu behandeln.

`columns[0]` kommt so oft vor, das würde ich z.B. `key` nennen.
PEP 8: amount und followers sollten klein geschrieben werden, aber das liegt vielleicht nicht in deiner Macht, falls die Klasse von extern kommt.

Der IndexError ist vermutlich für `columns[2]` gedacht, aber der Zugriff darauf wird ja schon vorher abgefangen und dann gar nicht ausgeführt. Der IndexError kann also gar nicht auftreten. (Bei columns[1] gäbe es übrigens ein ähnliches Problem.)

Das `temp` im Namen ist überflüssig.

Bedingungen für das `if` werden in Python nicht in Klammern gesetzt - es sei denn, es gibt einen guten Grund dafür. Hier nicht.

Das eigentlich Problem liegt vermutlich NetplanNode. Das sollten Attribute der Instanz sein, nicht der Klasse. (So etwas wie public gibt es nicht wirklich in Python).

Dann bleibt untenstehender code übrig.

Code: Alles auswählen

def load_plan(filepath):
        net_plan = dict()
        with open(filepath, 'r') as lines:
            for line in lines:
                columns = line.split(" ")
                key = columns[0]
                if key in net_plan:
                    raise ValueError('Loop has been found!')
                plan_node = NetplanNode()
                plan_node.amount = int(columns[1])
                if len(columns) > 2:  # ansonsten gibt es eben keine follower?
                    plan_node.followers.extend(follow.rstrip() for follower in columns[2].split(","))
                net_plan[key] = plan_node
        return net_plan

class NetplanNode(object):
    def __init__(self):
        self.amount = 0
        self.followers = list()
        ...
        self.critical = False

Aber alles ungetestet, offensichtlich.

[__blackjack__]

Noch eine Anmerkungen zum Entwurf: So etwas wie das laden von Dateien gehört eher nicht in eine `__init__()`-Methode, auch nicht indirekt durch einen Funktions- oder Methodenaufruf. Je komplexer eine `__init__()` um so schlechter lassen sich solche Objekte zum testen erstellen. In `__init__()`s versuche ich meistens nur Code zu packen der möglichst einfach die Attribute definiert, im Idealfall werden die einfach nur als Argumente übergeben und an das Objekt gebunden. ”Konstruktoren” die mehr tun, zum Beispiel die Daten aus einer Datei laden, definiere ich als Klassenmethoden. Das spielt dann auch sehr gut mit dem `attr`-Modul zusammen das ich gerne für Klassen verwende.

[__blackjack__]

Nachtrag: Ich trenne auch gerne Laden aus einer Datei und parsen von Zeilen in zwei Klassenmethoden auf, ebenfalls weil man dann leichter Tests schreiben kann. Fürs Parsen ohne das man da mit Dateien hantieren muss. Denn das mit dem ``''.join(line).split(" ")`` ist gar nicht so harmlos wie bords0 das angenommen hat: Da kommt eine Zeichenkette bei heraus. Das heisst columns[0] ist *ein* Zeichen, genau wie columns[1] nur ein Zeichen ist, und columns[2] auch, und spätestens da sieht man dass das falsch sein muss, denn es macht keinen Sinn da dann ``columns[2].split(",")`` zu schreiben, denn *ein* Zeichen an einem "," zu splitten und dann in einer Schleife über die Ergebnisse zu iterieren ist super sinnlos, denn wenn es ein Komma ist, dann bekommt man als Ergebnis zwei leere Zeichenketten, ansonsten bekommt man eine Liste mit genau einem Element — dem Zeichen selbst. Und da macht ein `rstrip()` nur Sinn wenn man aus einem Whitespace-Zeichen eine leere Zeichenkette machen will. So sehen die Eingabedaten aber ziemlich sicher nicht aus.

[bords0]

Denn das mit dem ``''.join(line).split(" ")`` ist gar nicht so harmlos wie bords0 das angenommen hat: Da kommt eine Zeichenkette bei heraus. Das heisst columns[0] ist *ein* Zeichen, genau wie columns[1] nur ein Zeichen ist, und columns[2] auch,

Da kommt doch eine Liste heraus und keine Zeichenkette?

Code: Alles auswählen

In [129]: line = "foo bar"

In [130]: columns = "".join(line).split()

In [131]: columns
Out[131]: ['foo', 'bar']

In [132]: columns[0]
Out[132]: 'foo'

[__blackjack__]

@bords0: Verdammt, stimmt.

[Matze27396]

Da ist einiges nicht so ganz pythonisch. Unter anderem:

Doppelte Unterstriche am Anfang sind nicht für "private" Attribute da, die gibt es nicht in Python. Einfach weglassen, bis du weißt, was sie bedeuten.
Namenskonventionen (PEP beachten, es sollte eher `def load_plan` heißen.
`self` ist unnötig, da nicht verwendet. Das sollte keine Methode einer Klasse sein (welcher?) sondern eine Funktion.

Wieder PEP 8: net_plan statt tempNetplan.

validatekeylist ist unnötig, da die keys ja schon im dictionary net_plan zur Verfügung stehen.

Die Ausnahmebehandlung ist m.E. nicht so günstig. Erstmal keine Ausnahmen abfangen, es sei denn, man kann was vernünfigtes damit machen. Eine print-Anweisung und Rückgabe eines unnützen Wertes ist meistens nichts vernünftiges - das könnte der Rufer der Funktion im Zweifel nämlich besser!
Deshalb die Datei mit `with open(filepath, 'r') as lines:` öffnen.
`''.join(line)` ist überflüssig, das ist das gleiche wie `line`.

`tempNetplan.clear()` löscht zwar das dict, hat aber den Rückgabewert `None`. Eine simples `return` würde also das gleiche bewirken. Aber wie gesagt ist es besser, die Ausnahme nicht explizit zu behandeln.

`columns[0]` kommt so oft vor, das würde ich z.B. `key` nennen.
PEP 8: amount und followers sollten klein geschrieben werden, aber das liegt vielleicht nicht in deiner Macht, falls die Klasse von extern kommt.

Der IndexError ist vermutlich für `columns[2]` gedacht, aber der Zugriff darauf wird ja schon vorher abgefangen und dann gar nicht ausgeführt. Der IndexError kann also gar nicht auftreten. (Bei columns[1] gäbe es übrigens ein ähnliches Problem.)

Das `temp` im Namen ist überflüssig.

Bedingungen für das `if` werden in Python nicht in Klammern gesetzt - es sei denn, es gibt einen guten Grund dafür. Hier nicht.

Das eigentlich Problem liegt vermutlich NetplanNode. Das sollten Attribute der Instanz sein, nicht der Klasse. (So etwas wie public gibt es nicht wirklich in Python).

Dann bleibt untenstehender code übrig.

Code: Alles auswählen

def load_plan(filepath):
        net_plan = dict()
        with open(filepath, 'r') as lines:
            for line in lines:
                columns = line.split(" ")
                key = columns[0]
                if key in net_plan:
                    raise ValueError('Loop has been found!')
                plan_node = NetplanNode()
                plan_node.amount = int(columns[1])
                if len(columns) > 2:  # ansonsten gibt es eben keine follower?
                    plan_node.followers.extend(follow.rstrip() for follower in columns[2].split(","))
                net_plan[key] = plan_node
        return net_plan

class NetplanNode(object):
    def __init__(self):
        self.amount = 0
        self.followers = list()
        ...
        self.critical = False

Aber alles ungetestet, offensichtlich.

Gibt es keine Privateattribute? Das habe ich auf anderen Seiten aber schon anders gelesen. Da steht es oftmals mit den Unterstrichen.
Erstmal Danke für deine Verbesserungsvorschläge. Werd ich mir mal durchschauen und dann so übernehmen wenns mir passt.
Das mit dem "join" hatte ich gar nicht so genau angeschaut ehrlich gesagt.

[Matze27396]

Nachtrag: Und ja der eigentliche Fehler lag wohl echt an den "Klassenattributen" die ich so nicht beabsichtigt hatte. Puhh....in Python ist echt vieles anders.

[ThomasL]

Gibt es keine Privateattribute? Das habe ich auf anderen Seiten aber schon anders gelesen. Da steht es oftmals mit den Unterstrichen.

Dann tummelst du dich auf den falschen Seiten. Diese Lesezeichen besser löschen.

[__blackjack__]

@Matze27396: Bezüglich ”private” hat man halt das Problem das so einiges an Material im Netz und selbst in Büchern über Python von Leuten stammt die von einer anderen Sprache kommen, und versuchen in Python Entsprechungen für das suchen was sie kennen und auf biegen und brechen ”public”, ”protected”, und ”private” auch in Python irgendwie sehen wollen. De facto gibt es keinen Zugriffsschutz, was diese drei Schlüsselworte in anderen Programmiersprachen steuern.

Zitat aus dem Tutorial in der Python-Dokumentation:

“Private” instance variables that cannot be accessed except from inside an object don’t exist in Python. However, there is a convention that is followed by most Python code: a name prefixed with an underscore (e.g. _spam) should be treated as a non-public part of the API (whether it is a function, a method or a data member). It should be considered an implementation detail and subject to change without notice.

Namen mit einem führenden Unterstrich werden in Python als Implementierungsdetail gesehen auf das man von aussen nicht zugreifen sollte wenn man nicht selbst der Autor ist oder genau weiss was man tut und mit den Konsequenzen leben kann.

Namen mit zwei führenden (und keinen folgenden) Unterstrichen werden von Python automagisch verändert, damit man bei Mehrfachvererbung oder tiefen Vererbungshierarchien keine Namenskollisionen hat. Da beides aber eher selten gemacht wird, werden diese Namen auch selten wirklich verwendet. Ausser eben von Javaprogrammieren Ein Zugriffsschutz ist das aber auch nicht, weil die Regeln nach denen die Namen verändert werden sehr einfach ist und in der Python-Dokumentation beschrieben ist. Im Grunde wird da nur der Klassenname in den Namen eingebaut:

Code: Alles auswählen

In [259]: class Spam: 
     ...:     def __method(self): ... 
     ...:                                                                       

In [260]: x = Spam()                                                            

In [261]: x._Spam__method                                                       
Out[261]: <bound method Spam.__method of <__main__.Spam object at 0x7f69cfc7be10>>

Die FAQ in der Python-Dokumentation hat dazu auch noch I try to use __spam and I get an error about _SomeClassName__spam.

[__blackjack__]

Hab mich auch mal an einer Umsetzung versucht, mit dem `attr`-Modul und einer `classmethod()`. Ungetestet:

Code: Alles auswählen

#!/usr/bin/env python3
from attr import attrib, attrs


@attrs
class NetplanNode:
    amount = attrib(default=0)
    followers = attrib(factory=list)
    ...
    critical = attrib(default=False)

    @classmethod
    def parse(cls, line):
        key, amount_text, *rest = line.split()
        rest.append("")
        return (
            key,
            cls(
                int(amount_text),
                [follower.strip() for follower in rest[0].split(",")],
            ),
        )


def _load_plan(file_path):
    netplan = dict()
    with open(file_path, "r", encoding="ascii") as lines:
        for key, node in map(NetplanNode.parse, lines):
            if key in netplan:
                raise ValueError("there is a loop")
            netplan[key] = node

    return netplan

[Matze27396]

Das Witzige ist ja das unser Professor die Umsetzung will ..."Schreiben Sie geeignete public, sowie private Methoden dazu(Zugriff ggf. über public getter/setter-Methoden)"
Ansonsten hätte ich das ganze ja ganz gelassen...auch wenn ich ja persönlich auch von der C#-Seite komm und das gerne aufteile.

[Matze27396]

Hab mich auch mal an einer Umsetzung versucht, mit dem `attr`-Modul und einer `classmethod()`. Ungetestet:

Code: Alles auswählen

#!/usr/bin/env python3
from attr import attrib, attrs


@attrs
class NetplanNode:
    amount = attrib(default=0)
    followers = attrib(factory=list)
    ...
    critical = attrib(default=False)

    @classmethod
    def parse(cls, line):
        key, amount_text, *rest = line.split()
        rest.append("")
        return (
            key,
            cls(
                int(amount_text),
                [follower.strip() for follower in rest[0].split(",")],
            ),
        )


def _load_plan(file_path):
    netplan = dict()
    with open(file_path, "r", encoding="ascii") as lines:
        for key, node in map(NetplanNode.parse, lines):
            if key in netplan:
                raise ValueError("there is a loop")
            netplan[key] = node

    return netplan

Sieht gut aus:D Werd das ganze leider nicht so verwenden...ist ja ne Studienarbeit. Muss mir das ganze ja selbst erarbeiten:D Aber danke:)

[bb1898]

Das Witzige ist ja das unser Professor die Umsetzung will ..."Schreiben Sie geeignete public, sowie private Methoden dazu(Zugriff ggf. über public getter/setter-Methoden)"
Ansonsten hätte ich das ganze ja ganz gelassen...auch wenn ich ja persönlich auch von der C#-Seite komm und das gerne aufteile.

Witzig ist nicht das Adjektiv, das mir hierzu zuerst einfällt. Oder hat er einfach seine Vorlesungsmanuskripte verwechselt?

[__blackjack__]

Vielleicht sollte dem dann mal jemand sagen er soll keine Aufgaben für Python stellen wenn er eigentlich Java haben will. Triviale getter/setter-Methoden sind ja auch total unpythonisch, weil es im Gegensatz zu Java `property()` gibt wenn man den Zugriff dann doch mal durch Methoden leiten will ohne das sich die API ändert. Das ist ja der Grund warum man diesen ganzen Boilerplate-Code in Jave schreibt.

Da wo der Professor ”private” haben will ist das im Python wie gesagt *ein* führender Unterstrich.

[Matze27396]

Vielleicht sollte dem dann mal jemand sagen er soll keine Aufgaben für Python stellen wenn er eigentlich Java haben will. Triviale getter/setter-Methoden sind ja auch total unpythonisch, weil es im Gegensatz zu Java `property()` gibt wenn man den Zugriff dann doch mal durch Methoden leiten will ohne das sich die API ändert. Das ist ja der Grund warum man diesen ganzen Boilerplate-Code in Jave schreibt.

Da wo der Professor ”private” haben will ist das im Python wie gesagt *ein* führender Unterstrich.

Hintergrund: Er ist eigentliche Medienprof und gibt Vorlesungen zur Appentwicklung(meist in Java ). Das erklärt einiges. Das "Python"-Modul haben Sie ihm wohl dieses jahr aufgedrückt.
Naja...dann lös ich das ganze wohl unschön und kassier die "Punkte" und behalt mir aber im Hinterkopf wie es eigentlich ist.

Danke nochmal für die vielen Tipps und Ratschläge. Hat mir sehr geholfen.