Bilder downloaden und umbenennen

Wenn du dir nicht sicher bist, in welchem der anderen Foren du die Frage stellen sollst, dann bist du hier im Forum für allgemeine Fragen sicher richtig.
Antworten
vogti
User
Beiträge: 31
Registriert: Mittwoch 21. Januar 2009, 21:53
Kontaktdaten:

Code: Alles auswählen

# -*- coding: cp1252 -*-
import urllib
import os
def speicher_bild(name,url):
    if os.path.exists('pics1/'+name+'.jpg') != True:
        urllib.urlretrieve(url, 'pics1/'+name+'.jpg')
    else:
        pass
    
def name_bild():
    name=''
    url=''
    datei = open('bilder', 'r')
    weiter = True
    while weiter:
        line = datei.readline()
        if len(line) == 0:
            weiter = False
            break
        else:
            daten = line.split('#!')
            name = daten[0]
            url = daten[1]
            print name, url
            if ((name) or (url) != ''):
                speicher_bild(name,url)
            else:
                pass

execute=name_bild()
Mit urllib funktioniert es zwar, jedoch würde ich lieber ein pycURL äquivalent nutzen (Leider kommt noch eine Fehlermeldung, wenn url leer ist. Weiß noch nicht genau, wie man das wegbekommt...). Danke BlackVivi für den Tip mit urlretrieve!

Übrigens wollte ich keineswegs irgendwen angreifen.
Benutzeravatar
str1442
User
Beiträge: 520
Registriert: Samstag 31. Mai 2008, 21:13

Nutze os.path.join zum Zusammensetzen von Pfaden, == oder != True / False ist unnötig, da sowieso der Ausdruck ausgewertet + bool() darauf angewendet wird und if somit einen Wahrheitswert bekommt, den man nicht nochmal auf True prüfen muss, weiter als Variable ist bei Nutzung von break unnötig (while True: ...), den else Block im if in name_bild würd ich weglassen und einfach mit break aus dem Code springen (wie du es eh schon machst). Und (nach PEP8) zwischen dem "=" bei Zuweisungen außerhalb von Funktions-Keyword-Argumenten immer ein Leerzeichen, sofern du dich daran halten willst. (Kann sowieso nicht verstehen, warum Code manchmal so fürchterlich unaufgeräumt und inkonsistent aussieht - mich würd das selber ziemlich nerven (@vogti, dein Code ist nicht gemeint ;))). Ach, und Namen, die man nur (zur Klarheit) "bekannt machen" will, würd ich einfach None zuweisen anstatt eine leere Instanz der entsprechenden Klasse. Dann gibts auch nen Error, wenn man mal einen vergisst und None irgendwo herumfliegen hat.
BlackJack

@vogti: Ergänzend zu str1442: Ich würde Namen gar nicht "bekannt machen", sondern erst dort verwenden, wo sie auch wirklich gebraucht werden. Sollte das zu unübersichtlich werden, ist das oft ein gutes Zeichen dafür, dass man zu viele lokale Namen hat und eventuell die Funktion aufteilen sollte.

``pass`` in ``if``/``else``-Konstrukten ist überflüssig. Das kann man immer auch ohne ``pass`` ausdrücken. In diesem Fall kann man die ``else``-Zweige einfach weglassen.

Man sollte Wiederholungen im Quelltext vermeiden. Den Dateinamen braucht man nur einmal zusammensetzen.

Dateien, die man öffnet, sollte man auch wieder schliessen. Am besten in einem ``finally``-Block oder mit einer ``with``-Anweisung um sicher zu gehen.

Die ``while``-Schleife drückt man "pythonischer" als ``for``-Schleife über das Dateiobjekt aus. Dateien sind "iterable". Dann kann man sich auch die Abbruchbedingung mit dem ``break`` sparen. Die ``for``-Schleife endet automatisch am Dateiende.

`daten` kann man sich sparen, wenn man das Ergebnis von `split()` gleich den richtigen Namen zuweist.

Die Bedingung vor dem Aufruf von `speicher_bild()` ist sehr eigenartig geklammert. Die kann man alle weglassen, ohne dass sich etwas ändert. Dann bleibt aber immer noch der Eindruck, dass Du da etwas falsch verstehst. Es werden dort nicht `name` und `url` explizit mit ``!= ''`` verglichen, sondern nur `url`. Das macht nur deshalb keinen Unterschied weil ``if name`` im Grunde das gleiche ist wie ``if name != ''``. Zeichenketten sind "falsch" wenn sie leer sind und "wahr" sonst. Ich weiss auch nicht ob ``or`` die richtige Verknüpfung ist, denn ich hätte hier eher erwartet, das gespeichert werden soll, wenn `name` *und* `url` nicht leer sind!?

`name_bild()` hat keinen Rückgabewert, es macht also wenig Sinn das implizite `None` an `execute` zu binden.

Letztendlich sähe das Ganze dann so aus (ungetestet):

Code: Alles auswählen

def speicher_bild(name, url):
    jpeg_name = os.path.join('pics1', name + '.jpg')
    if not os.path.exists(jpeg_name):
        urllib.urlretrieve(url, jpeg_name)

def name_bild():
    with open('bilder', 'r') as datei:
        for line in datei:
            name, url = line.split('#!')
            print name, url
            if name and url:
                speicher_bild(name, url)

name_bild()
Antworten