Viele verschachtelte Schleifen und PEP8

[würmchen]

Hi Leute,
ich wollt euch mal fragen wie ihr das macht, wenn ihr einfach mit den 80 Zeichen nicht auskommt. PEP8 gibt ja vor möglichst nicht mehr als 79 Zeichen zu nutzen und dann mit einem Backslash umzubrechen.

Ich hab jetzt gerade eine Aufgabe in der ich 8 Schleifen habe. Die ganzen sind dann in einer Funktion in einer Klasse definiert und somit schonmal von vornherein 8 Leerzeichen eingerückt. In der letzten Schleife hab ich dann nur noch 36 Zeichen übrig wenn ich mich an PEP8 halten will.

Ich würde mich mal über euren Rat freuen.

[audax]

du solltest einfach keine 8-fach geschachtelten Schleifen nutzen.

[würmchen]

Also das Problem ist das die Daten leider so geschachtelt vorliegen.
Ich dachte auch schon ein eine Funktion die praktisch die Schleife darstellt (versteht man das).
Wollte halt mal die Tipps von den Fachmenschen hören...

[EyDu]

Die einzelnen Verschachtelungsebenen werden sicherlich irgend eine Bedeutung haben, daran kannst du dich beim Entwurf deiner Funktionen etwas orientieren. Im Normalfall wirst du sicher nicht mehr als zwei bis drei Verschachtelungen pro Funktion brauchen.

[BlackJack]

Bei so etwas bieten sich Generatorfunktionen an.

So eine tief verschachtelte Datenstruktur, bei der man in *einer* Funktion komplett durch greift, statt die entsprechenden Methoden auf den Objekten auf zu rufen könnte ein "code smell" sein.

[epsilon]

Also das Problem ist das die Daten leider so geschachtelt vorliegen.

Kannst du mal ein Beispiel für die Daten posten?

[würmchen]

Ich frage MySQL Tabellen ab, die Daten sind in insgesamt 93 Tabellen verteilt.

Beipiel, ein Entry kann mehrere Modelle haben, ein Modell kann mehrere Proteinketten haben, jede Proteinkette besteht aus mehreren Residues, Jedes Residue aus mehreren Atomen, Jedes atom hat 3 Koordinaten.

Ganz grob die Struktur....

Sind riesige Tabellen und ich denke es macht nicht so viel Sinn hier ausschnitte zu posten.



Ich bin gerade dabei mir mehr über Generatorfunktionen anzueigenen...

[audax]

Dann Teil das in Unteraufgaben auf und pack die in eigene Funktionen

[Mad-Marty]

Aufteilen zum einen, und die indent size auf 2 hilft

Jetzt kommt bestimmt noch ein entrüstetes "not 4 spaces-posting"

[würmchen]

ok, so bin ich gerade dabei das zu machen, und jede funktion ruft dann eben eine andere funktion nochmal auf...

ich benenne die gerade wie ich die schleifenköpfe beschreiben würde...

Code: Alles auswählen

def loopMolecule:
    ....
    loopResidue()

def loopModel:
    ....
    loopMolecule()

[CM]

Beipiel, ein Entry kann mehrere Modelle haben, ein Modell kann mehrere Proteinketten haben, jede Proteinkette besteht aus mehreren Residues, Jedes Residue aus mehreren Atomen, Jedes atom hat 3 Koordinaten.

Das ist ja eine ganz normale Datenstruktur in der Strukturbiologie (NMR-Daten?). Wieso ist denn so was *so* in einer Datenbank? Da muß es ja erstmal reingekommen sein. Und so umständlich ist doch gar nicht notwendig: Es kann ja eine Eingabe mit einer Datei assoziiert sein und dann kann ein normaler Parser mit dem Text dieser Datei (PDB, mmCIF, das eine oder ander XML-Derivat, etc.) arbeiten.

Jedenfalls: Eine Funktion, bzw. eine Schleife, für jede Hierachieebene ist tierisch umständlich und dauert im Zweifel lange.

Gruß,
Christian

[audax]

ok, so bin ich gerade dabei das zu machen, und jede funktion ruft dann eben eine andere funktion nochmal auf...

ich benenne die gerade wie ich die schleifenköpfe beschreiben würde...

Code: Alles auswählen

def loopMolecule:
    ....
    loopResidue()

def loopModel:
    ....
    loopMolecule()

Das ist leider ebenso keine gute Idee :/
Du benutzt da globale Variablen, oder?
Zeig einfach deinen Code...

[würmchen]

Beipiel, ein Entry kann mehrere Modelle haben, ein Modell kann mehrere Proteinketten haben, jede Proteinkette besteht aus mehreren Residues, Jedes Residue aus mehreren Atomen, Jedes atom hat 3 Koordinaten.

Das ist ja eine ganz normale Datenstruktur in der Strukturbiologie (NMR-Daten?). Wieso ist denn so was *so* in einer Datenbank? Da muß es ja erstmal reingekommen sein. Und so umständlich ist doch gar nicht notwendig: Es kann ja eine Eingabe mit einer Datei assoziiert sein und dann kann ein normaler Parser mit dem Text dieser Datei (PDB, mmCIF, das eine oder ander XML-Derivat, etc.) arbeiten.

Jedenfalls: Eine Funktion, bzw. eine Schleife, für jede Hierachieebene ist tierisch umständlich und dauert im Zweifel lange.

Gruß,
Christian

ganz recht, es dreht sich um eine datenbank in der rund 30000 mmCIF gespeichert sind. der code der die datenbank erstellt hat stammt nicht von mir, ich arbeite nur mit der datenbank. jetzt geht es darum praktisch wieder ein mmCIF bzw PDB zu schreiben und das noch mit verschiedenen einschränkungen bzw zusätzlichen Infos


@audax
ich bin noch am bauen, aber so sieht der verschachtelte code aus der fast fertig ist
die ganzen getXyz() funktionen sind in der Klasse von der Ich ableite definiert.

Code: Alles auswählen

class CreateAllPdbFiles(CreatePdbFiles):
    def __init__(self,cursor,entry_key_string):
        CreatePdbFile.__init__(self,cursor)
        self.entry_key_list = entry_key_string.split()

        for entry_key in self.entry_key_list:
            pdb_obj = self.getPdbCode(entry_key)
            pdb_obj['model'] = self.getModelInfo(entry_key)
            pdb_file = pdb(name=pdb_obj['pdb_code']+'.pdb')
     
     
            for model_key,model_id in pdb_obj['model']:
                if len(pdb_obj['model']) > 1:
                    newmodel = ['MODEL',str(model_id)]
                    pdbfile.addAtom(newmodel)
     
                pdb_obj['molecule'] = self.getMoleculeInfo(entry_key,model_key)

                for molecule_key,moleculetype in pdb_obj['molecule']:
                    my_atom_to_atom = {}
                    bonds[]

                    pdb_obj['residue'] = getResidueInfo(entry_key,molecule_key)
                    for redidue_key,resName,resSeq,iCode,heteroFlag,chainID \
                            in pdb_obj['residue']:

                        if self.only_catres:
                            pdb_obj['catres'] = getCatResInfo(entry_key)
                            if not residue_key in \
                                    [i[2] for i in pdb_obj['catres']]:
                                        continue
                        hetero = "ATOM"    
                        if heteroFlag.strip(" ") != "": 
                            hetero = "HETATM"

                        # Residue filter
                        if skip_all_hetero:
                            if hetero == "HETATM":
                                continue
                        if only_nat_residues:
                            if resName not in nat_residues:
                                print 'Warning from getPdb::protein2pdb: ' +
                                'Residue was ' + str(resName)
                                continue

                        # Get atom info
                        pdb_obj['atom'] = self.getAtomInfo(entry_key)
     
                        for atom in [i for i in pdb_obj['atom'] \
                                if i[3] == residue_key]:
                            pass

[CM]

Oh . Also, ich nehme mal an, andere hier können Dich da noch eher auf den Senkel stellen und ggf. auch zeigen, wie man effizienter durch die Datenbank crawlt.

Aber zunächst einmal sollte man nicht so viel Funktionalität in die __init__-Funktion einer Klasse stecken. Dann würde ich auch nicht eine Klasse nach ihrer wesentlichen Funktionalität benennen (sehr seltsame Herangehensweise).

Vor allem aber: Du kennst biopython (besitzt die Möglichkeit ein PDB-File zu schreiben, wenn Du einmal ein PDB-Objekt erstellt hast)? Oder openbabel (damit kannst Du ebenfalls PDB-Files schreiben und parsen und auch mmCIF-Files erstellen - besitzt auch eine Pythonanbindung)?

Gruß,
Christian

[snafu]

Muss man hier überhaupt mit einer Klasse arbeiten?

[würmchen]

Der Ansatz mit der Klasse und der __init__ hatte ich desshalb, weil ich eine liste mit keys bekomme (jeder key stellt ein bestimmtes pdb dar) und ich dazu dann mit bestimmten kriterien pdb's erstelle. Der Ablauf aber immer der selbe ist. Ich dachte dann brauch ich nicht nochmal eine extra Funktion zu definieren. Ich kann auch nochmal eine Funktion bestimmen die die Daten letzendlich holt, aber dachte das kommt dann aufs selbe raus. So spare ich mir später eine Zeile beim Objekt erzeugen.

Biopython kenne ich. Ich importiere auch teilweise Pakete davon.
Da ich aber ein modifiziertes PDB schreibe kann ich nicht so viel davon verwenden oder konnte es bis jetzt noch nicht nach meinen Vorstellungen umbauen.


@snafu
Welche Nachteile bringt es denn mit sich?

[Y0Gi]

Eine gute Idee ist es bereits, nicht etwas in große if-Blöcke zu stecken, sondern mit einer gegenteiligen Abfrage vorzeitig aus dem umgebenden Block zu springen (``if not foo: continue``) - das machst du ja bereits.

Mehrere if-Verschachtelungen kannst du auch zusammenfassen, (``if foo and (bar == 'tralala'): ...``), was jedes Mal mindestens eine Einrückungsebene spart.

Zudem empfehle ich dir zu prüfen, ob du nicht gewinnbringend ``filter`` bzw. ``itertools.ifilter``/``itertools.ifilterfalse`` und weitere funktionale Konstrukte einsetzen kannst. Das scheint sich mir bei deinem Code zu lohnen. So kannst du im Vorfeld Elemente aussortieren und hast weniger Logik in einem evtl. verbleibenden "Hauptloop".

[CM]

@snafu
Welche Nachteile bringt es denn mit sich?

Ich hatte etwas Ähnliches wie snafus Frage bereits aus meinem Post gestrichen. Also ich finde eine Klasse hier auch nicht unbedingt gerechtfertigt. Nachteile? Nicht unbedingt. Aber was sind die Vorteile etwas in eine Klasse zu packen, dass da semantisch nicht hin gehört? Das das da "nicht hin gehört" schreibst Du selber durch den Namen Deiner Klasse, der sagt "ich bin eine Funktion und mache etwas" und nicht "ich bin eine Klasse und enthalte etwas". Aber vielleicht liegt mein Empfinden nur daran, dass ich den Rest vom Code nicht kenne.

Was ich machen würde ist eine Modell-Klasse zu entwerfen, deren Objekte sich ggf. aus der Datenbank initialisieren und Schreibmethoden für Deine Ausgabefiles haben.

Gruß,
Christian[/url]