fug - ein einfacher Webcomic-Downloader

Stellt hier eure Projekte vor.
Internetseiten, Skripte, und alles andere bzgl. Python.
Antworten
webspider
User
Beiträge: 485
Registriert: Sonntag 19. Juni 2011, 13:41

Vielleicht wolltet ihr ja schon immer mal sämtliche Ausgaben von Megatokyo oder dergleichen lesen, aber hattet keine Lust euch durch die Webseite zu klicken. fug (wie in Unfug) lädt sämtliche Bilder von Anfang bis Ende herunter und nutzt Module für die von Webcomic zu Webcomic unterschiedliche Funktionalität (frei nach "Special cases aren't special enough to break the rules." interpretiert).

Dies ist mein erstes Projekt dass ich mittels hg auf aktuellem Stand halte. Ich habe vor als nächstes sowohl Behandlung von einfachen requests-Ausnahmen (sei es ein 404-Error wegen eines fehlenden Bilds oder aktuell schlechter Internetanbindung) als auch Umbenennen der Bilder nach einem vorgegebenem Schema umzusetzen. Bei letzterem weiß ich nicht wie ich es möglichst sauber implementiert bekomme. Gedacht habe ich an Pythons neue format-Strings und ein Dictionary mit allen aktuellen Werten (id, originaler Name).

Sollte es mehr zum Beanstanden geben, würde ich mich über Kritik sehr freuen. Vielen Dank :mrgreen:
webspider
User
Beiträge: 485
Registriert: Sonntag 19. Juni 2011, 13:41

Sagt bloß der Code ist akzeptabel? Egal, das habe ich nun geändert und eine ziemlich grässliche Behandlung von 404-Errors und dergleichen eingefügt. Ich mache mich derweil an das andere Feature, vielleicht wisst ihr ja wie man es geschickter lösen kann.
lunar

Ich finde den Quelltext recht gut, würde aber selbst ein paar Dinge anders machen.

So würde ich nicht prüfen, ob ein Plugin wirklich alle Funktionen bereitstellt. Diese Prüfung ist nicht besonders aussagekräftig, garantiert sie doch nicht, dass die Funktion auch richtig und vollständig implementiert sind? Was beispielsweise, wenn alle Funktionen vorhanden sind, aber "NotImplementedError()" werfen, eine typische Vorgehensweise, um Funktion als "noch zu implementieren" zu markieren? Dein Quelltext lädt das Plugin anstandslos, und schlägt später trotzdem fehlt. Verwende das Plugin einfach ohne gesonderte Überprüfung. Wenn es nicht korrekt ist, erhältst Du schon entsprechende Ausnahmen, i.e. einen AttributeError. Ohnehin solltest Du den mitgelieferten Plugins doch eigentlich auch vertrauen können, dass sie korrekt und vollständig sind, oder?

Ich würde die Plugins auch nicht manuell über "imp" laden. Python hat doch bereits einen funktionierenden Import-Mechanismus, verwende diesen. Füge das Plugin-Verzeichnis "sys.path" hinzu, oder besser, wandle Dein Programm in ein Paket um, und importiere die Plugins dann einfach aus dem Paket "fug.plugins" mithilfe von "__import__()".

Die ganzen "print()"s in den verschiedenen Funktionen würde ich entfernen. Nutze "logging" für Ausgaben. Damit kannst Du dem Benutzer auch über Kommandozeilenargumente die Möglichkeit geben, die Ausführlichkeit der Ausgabe zu bestimmen.

Die Plugins könnten Klassen sein, damit Du von Vererbung profitieren kannst. Die Notwendigkeit von Vererbung zeigt Dein "base"-Plugin, das eigentlich eine abstrakte Basisklasse für alle Plugins sein könnte.

Das Speichern in eine Datei ist imho auch eigentlich keine Plugin-Funktionalität. Lass die Plugins die Bilder als Bytestrings oder als Datei-Objekt zurückgeben, und implementiere das Speichern in "fug.py". Das vereinfacht die Plugins, macht unter anderem "base.py" überflüssig, und eröffnet Dir die Möglichkeit, das Speichern zu erweitern, beispielsweise eine Fortschrittsanzeige für den Download einzuführen, alle Bilder direkt in ein Archiv zu verpacken, etc.
webspider
User
Beiträge: 485
Registriert: Sonntag 19. Juni 2011, 13:41

- Ich habe schon ein wenig angezweifelt inwiefern eine Überprüfung in einer Programmiersprache wie Python Sinn ergibt. Den Vorschlag werde ich beherzigen und wohl später als Ausgleich Dokumentation für die Struktur eines Plugins hinzufügen für andere.
- __import__ schien mir ein wenig magisch zu sein, daher imp. Um ein Paket zu haben, reicht doch die Existenz von __init__.py aus, richtig?
- Vielen Dank auf den Hinweis auf logging, ich kannte das Modul so noch nicht.
- Ich denke man kann das so beschriebene Grundgerüst dann gleich mit dem Werfen eines NotImplementedError kombinieren und hätte so eine viel einfachere grobe Kontrolle für den Plugin-Schreiber als beim vorigen Ansatz. Darüber hinaus wäre dann eine davon abgeleitete generische Klasse möglich welche >95% der Webcomics abdeckt, der Rest sind dann Fälle wie Prequel, welche wohl einen kleinen Webcrawler erfordern.
- Eigentlich wollte ich alles an Funktionalität die Module außerhalb der Standardbibliothek erfordert von fug.py trennen, aber jetzt da ich Fehlerbehandlung mit den Exceptions von requests eingebaut hab, geht das nicht mehr.

Vielen Dank für die Kritik. Ich denke noch nach ob es Sinn machen würde zunächst den ganzen Bereich abzugehen und alle Bild-URLs für die Verarbeitung in einer Liste zu speichern. Das würde ermöglichen diese als Textdatei zu speichern falls man das Herunterladen lieber einem Werkzeug wie wget überlassen will. Über das Umbenennen der Dateien rätsele ich noch.
Benutzeravatar
snafu
User
Beiträge: 6744
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Was willst du denn mit diesem

Code: Alles auswählen

else:
    pass
in Zeile 83/84 der `fug.py` bezwecken? `pass` nutzt man eigentlich nur, wenn ein Block aus syntaktischen Gründen eingeleitet werden muss, aber in dessen Körper nichts weiter passieren soll. Ein prominentes Beispiel wäre das "Ignorieren" einer geworfenen Ausnahme. Die fängt man halt ab, aber führt keine darauf beruhende Aktion aus. Wobei genau genommen das Nichtstun natürlich auch eine Art von Handlung darstellt. Zu dem `else:` bist du an der Stelle aber nicht gezwungen, sodass man die beiden Zeilen prinzipiell auch weglassen kann. Oder dient dir das `pass` vielleicht als eine Art Platzhalter, der später ersetzt werden soll?
webspider
User
Beiträge: 485
Registriert: Sonntag 19. Juni 2011, 13:41

Der gesamte Abschnitt (aktuell Zeilen 70 - 89) mit dieser "Fehlerbehandlung" ist alles andere als gut gelöst und soll ein temporärer Platzhalter für eine bessere Umsetzung sein. Ich habe in den Weiten des Internets kaum Material zu dem Thema gefunden (wenn man vom Testen welche Ausnahmen auftreten und dem Schreiben eines passenden try-except-Konstrukts absieht). Vielleicht reicht es ja schon die Struktur der Schleife nochmals zu überdenken (funktionieren prinzipiell nicht Iteratoren/Generatoren ähnlich?) und so als Nebenprodukt eine bessere Lösung um auftretende Ausnahmen zu behandeln zu erhalten.
Benutzeravatar
snafu
User
Beiträge: 6744
Registriert: Donnerstag 21. Februar 2008, 17:31
Wohnort: Gelsenkirchen

Ich würde ja vorschlagen, das Ganze nochmal neu zu schreiben und dann objektorientiert zu implementieren. Folgende grobe Struktur käme mir da in den Sinn:

Code: Alles auswählen

class ComicPage(object):
    def __init__(self, url, xpaths):
        self.url = url
        self.tree = etree.parse(urlopen(url))
        self.xpaths = xpaths

    def _fetch(self, name):
        return self.tree.xpath(self.xpaths[name])[0]

    def fetch_prev_page(self):
        url = self._fetch('prev')
        return ComicPage(url, self.xpaths)

    def fetch_next_page(self):
        url = self._fetch('next')
        return ComicPage(url, self.xpaths)

    def fetch_image(self):
        url = self._fetch('img')
        return ComicImage(url)
Ein `ComicImage` könnte dann wieder Dinge wie `.save_as()` oder `.show()` implementieren, wenn man denn (irgendwann) soweit gehen möchte. Wahrscheinlich reicht es für den Anfang erstmal, die URL statt eines eigenen Objektes auszugeben.

Und dann müsste man sich natürlich noch überlegen, was passiert, wenn es in eine Richtung seitenmäßig nicht mehr weitergeht. Denkbar wären z.B. die Rückgabe von `None` oder das Werfen einer Ausnahme. Oder die Implementierung von `has_next()` / `has_prev()`. Ich würde wohl `None` nehmen und einen Iterator um die Sache schreiben.

Die Idee des Projektes finde ich auf jeden Fall sehr interessant. Evtl schreib ich dafür auch mal eine eigene Ausarbeitung. So nach meiner Vorstellung der API...
webspider
User
Beiträge: 485
Registriert: Sonntag 19. Juni 2011, 13:41

Es ging mittlerweile nach einer längeren Pause einen guten Schritt weiter und alle Kritikpunkte bis auf das Nutzen von Klassen/mehr Objektorientierung sind adressiert. Renaming-Support ist ebenfalls drin und es wurde nach Python3 portiert.

Ich warte noch das Release des nächsten LXML-Pakets für Arch ab, da dies es mir erlaubt lxml.html.fromstring mit der Ausgabe von requests.get.content zu verwenden und so die Fehlerbehandlung von requests zur Verfügung zu haben. Bis dahin hoffe ich einfach, dass keine Fehler bei lxml.html.parse auftreten, da ich keine wirklich fangbaren Ausnahmen dafür gesehen hab. Vielleicht hat ja auch jemand dafür eine Lösung parat. Ansonsten bleiben mir ja noch die Klassen-Geschichten, mehr Plugins und evtl. Dokumentation für potenzielle Pluginschreiber.

edit: Bin auf Klassen umgestiegen. lxml scheint bei Verbindungsfehlern beim Parsen immer einen OSError zu werfen. Ich warte besser noch ein Weilchen und schreibe in der Zwischenzeit Plugins.
webspider
User
Beiträge: 485
Registriert: Sonntag 19. Juni 2011, 13:41

Auto-Nekrobump!

Ich habe nach längerer Pause weitergearbeitet und stecke aktuell beim Paketieren fest. Da es Python3 ist, habe ich mich für Distribute entschieden, paketiert wird auch anscheinend erfolgreich, aber das erstellte Hauptskript welches der Nutzer verwenden soll ist nicht korrekt. Der Fortschritt ist auf Bitbucket zu sehen, hier ist die setup.py. Beim Ausführen von fug in einem virtualenv, gibt es folgende Fehlermeldung.

Gehe ich überhaupt idiomatisch vor? Oder wäre andere Paketierung angebrachter? Ich blicke nicht wirklich durch.

edit: Habe mir Distribute nochmal genauer angesehen und bemerkt, dass meine Syntax nicht korrekt war. Statt ``entry_points = {'console_scripts': ['fug = fug.main']}`` sollte es ``entry_points = {'console_scripts': ['fug = fug.main:main']}`` sein, da nach dem Gleichheitszeichen erst das Modul, dann nach einem Doppelpunkt die aufzurufende Funktion kommen soll. Mit etwas mehr Metadaten werde ich dann gleich die setup.py hochladen und mich nach mehr Testen an ein Archlinux-Paket machen.
Antworten