Stilfrage

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.
Benutzeravatar
Mr_Snede
User
Beiträge: 387
Registriert: Sonntag 8. Februar 2004, 16:02
Wohnort: D-Dorf, Bo

Stilfrage

Beitragvon Mr_Snede » Sonntag 6. Februar 2005, 00:37

Hallo,
Ich habe mir ein Script zusammengebaut, und es funktioniert sogar so wie ich es will.
Trotzdem habe ich einige Fragen denn dieses Skript soll weiteren Übungen dienen.

1) Was haltet ihr von den Namen meiner Variablen und den Docstrings ?

2) Sollte ich lieber die zwei Zeilen:

Code: Alles auswählen

    file_no_ext = os.path.splitext(inputpic)[0]
    file_full_path = os.path.join(pic_search_path , inputpic

in die Funktion "every_image_in_folder()" packen und dann als Parameter an "create_thumb()" übergeben?

Cu Sebastian



Code: Alles auswählen

#!/usr/bin/python
#Version (0.01)

import sys, os, Image

pic_search_path="/home/sabba/in/pic/"
thumb_height=100


def create_thumb(inputpic, max_h):
    """ creates a thumb of a pic by a given height with original aspectratio
        Paramters:
            inputpic = full path to pic
            max_h =  (in pixel) used to calculate new_w by aspectratio of inputpic
    """
    file_no_ext = os.path.splitext(inputpic)[0]
    file_full_path = os.path.join(pic_search_path , inputpic)

    im = Image.open(file_full_path)
    w, h = im.size
    new_w = max_h * w / h
    im.thumbnail((new_w, max_h), Image.ANTIALIAS)
    im.save(pic_search_path + file_no_ext + "-thumb.jpg", "JPEG")

    print "Thumb created:"
    print "--> " + pic_search_path + file_no_ext + "-thumb.jpg"
    print " "


def every_image_in_folder(inputfolder):
    """ walks through a given folder and calls create_thumb() for every image
        Paramters:
            inputfolder = folder with original pics
    """
    for i in os.listdir(inputfolder):
        create_thumb(i, thumb_height)


def main():
    every_image_in_folder(pic_search_path)

main()
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Sonntag 6. Februar 2005, 17:22

Also ich würde als zeite Zeile nach dem Shebang ein Encoding angeben:

Code: Alles auswählen

# -*- encoding: latin-1 -*-

ist oft bei spätererer Verwendung von Unicode sinnvoll.
Dann lasse ich um das '=' immer links und rechts ein Leerzeichen (mehr oder weniger konsequent zu sowas wie create_thumb(inputpic, max_h), wo ja auch zwischen den Argumenten ein Komma und ein Leerzeichen drin ist. Außerdem sind die meisten Python Quellcodes so formatiert. Daneben noch: statt die Version als Kommentar anzugeben, würde ich ein

Code: Alles auswählen

__version__ = 0.01
oder 0.0.1 machen, somit kann man es checken wenn man das Script vielleicht als Modul importiert oder in der Versionsinfo. Stören tut das nie. Zum Schluss würde ich statt einem puren main() lieber

Code: Alles auswählen

if __name__ == '__main__':
    main()

machen, hat auch eine Begründung.

Wenn du die zwei Zeilen irgendwohin packen willst, solltest du noch eine schließende Klammer anbringen, sonst verschluckt sich die Python ;)
My god, it's full of CARs! | Leonidasvoice vs Modvoice
BlackJack

Beitragvon BlackJack » Sonntag 6. Februar 2005, 23:54

1) Einige Namen sind für meine Geschmack zu kurz. Besonders das `i` in `every_image_in_folder()` gibt nicht unbedingt wieder worum es sich handelt.

Die maximale Höhe könnte man vielleicht als Default-Wert in der `create_thumb()` Funktion setzen, also

Code: Alles auswählen

create_thumb(image_filename, max_height=100)


2) Das mit der globalen Variable für das Verzeichnis ist nicht so schön. Vor allem weil Du sie an zwei Stellen unabhängig voneinander benutzt. Die `create_thumb()` Funktion ist alleine gar nicht benutzbar, weil der Pfad dort quasi hart reinkodiert ist.

Was sonst noch auffällt, ist dass nirgends darauf reagiert wird, was passiert wenn es sich gar nicht um eine Bild-Datei handelt.
Benutzeravatar
Mr_Snede
User
Beiträge: 387
Registriert: Sonntag 8. Februar 2004, 16:02
Wohnort: D-Dorf, Bo

Beitragvon Mr_Snede » Montag 7. Februar 2005, 18:47

Danke euch zwei.

Das Encoding habe ich nur aus Schusseligkeit vergessen.
Ui - die Leerzeichen habe ich wirklich nicht beachtet.
Leerzeichen um ein "=" und eines nach einem "," würde mir gefallen.
__version__= xyz finde ich auch toll.

Code: Alles auswählen

if __name__ == '__main__':
    main()

Stand schon auf meiner todo-Liste.

Nur wo soll ich noch eine Schließende Klammer hintun habe ich eine vergessen?

@BlackJack:
Bei dem i in der For-Schleife habe ich auch lange überlegt, und mich für das i entschieden, da es sozusagen der Standartname ist, den man in vielen Dokus und Beispielen findet.
Und weil bis jetzt ja einfach über alle Dateien im Ordner iteriert wird.
Mit der Abfrage, ob es sich um ein Bild handelt, finde ich einen bezeichnenden Namen sinnvoll. Soweit meine Überlegungen dazu.

Die maximale Höhe als Default-Wert in der `create_thumb()`- das finde ich gut, da wäre ich nicht drauf gekommen.

Die globalen Variablen sind mir auch ein Dorn im Auge und vor allen Dingen dass ich auf diese direkt zugreife.
Das sind Relikte vom Aufbohren und zusammenfügen von Beispielprogrammen.
In Zukunft soll zB. "pic_search_path" als Default das Verzeichnis enthalten aus dem das Programm gestartet wurde.
Daneben soll es als Parameter beim Programmstert mit übergeben werden können (genau wie "thumb_height") oder über eine GUI beeinflusst werden.

Da ist also noch einiges zu tun - vielleicht habe ich heute Abend ja etwas Zeit.
cu Sebastian
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Montag 7. Februar 2005, 19:23

Mr_Snede hat geschrieben:Nur wo soll ich noch eine Schließende Klammer hintun habe ich eine vergessen?

In diesem Code den du vorgeschlagen hast:

Code: Alles auswählen

file_full_path = os.path.join(pic_search_path , inputpic


Mr_Snede hat geschrieben:Bei dem i in der For-Schleife habe ich auch lange überlegt, und mich für das i entschieden, da es sozusagen der Standartname ist, den man in vielen Dokus und Beispielen findet.

Ich würde trotzdem in finalen Programmen nicht i nutzen, sondern etwas bezeichnendereres.
My god, it's full of CARs! | Leonidasvoice vs Modvoice
Benutzeravatar
Dookie
Python-Forum Veteran
Beiträge: 2010
Registriert: Freitag 11. Oktober 2002, 18:00
Wohnort: Salzburg
Kontaktdaten:

Beitragvon Dookie » Montag 7. Februar 2005, 19:50

Hi,

mal eine kleine Anmerkung zum i in Forschleifen. Historisch gesehen steht das i für Index, das kommt noch auch Zeiten, in denen Forschleifen einfach nur Zahlen hoch oder runter zählen konnten (z.B. in Basic), und dieser Zähler i dann als Index in ein Array oder einen String(Zeichenarray) verwendet wurde.

Da bei Python die Forschleife eigentlich eine Foreachschleife ist, sollte man dort besser einen den Objekten in dem Iterable über das mit For Iteriert wird entsprechenderen Namen verwenden. Gerade bei einer Schleife wie oben, bietet sich z.B. filename oder imagename an. Dafür spart man sich dann viel dokumentation da dann der Inhalt der Variable die in der Forschleife verwendet wird ohnehin offensichtlicher wird.


Gruß

Dookie

Code: Alles auswählen

#!/usr/bin/env python
import this
Benutzeravatar
Mr_Snede
User
Beiträge: 387
Registriert: Sonntag 8. Februar 2004, 16:02
Wohnort: D-Dorf, Bo

Beitragvon Mr_Snede » Montag 7. Februar 2005, 22:18

@Leonidas:
Ich habe die fehlende Klammer im Quelltext gesucht.
In Zukunft werde ich auf solche Fehler durch copy'n paste verstärkt achten.



Und die Variable in der Schleife bekommt gleich einen aussagekräftigen Namen.
Mal schauen, wie weit ich heute Abend noch komme.

cu Sebastian
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Montag 7. Februar 2005, 23:35

Mr_Snede hat geschrieben:Mal schauen, wie weit ich heute Abend noch komme.

Na dann musst du dich beeilen, dass es nicht morgen morgen wird ;)
My god, it's full of CARs! | Leonidasvoice vs Modvoice
Benutzeravatar
Mr_Snede
User
Beiträge: 387
Registriert: Sonntag 8. Februar 2004, 16:02
Wohnort: D-Dorf, Bo

Beitragvon Mr_Snede » Dienstag 8. Februar 2005, 12:57

@ leonidas:
Na dann musst du dich beeilen, dass es nicht morgen morgen wird

Nicht wirklich, ich habe die Sicht eines Studenten auf die Tageszeit:
Der Tag ist erst rum wenn ich ins Bett gehe :-)

Hier ist das was ich soeben verbrochen habe:

Code: Alles auswählen

#!/usr/bin/python
# -*- encoding: latin-1 -*-

import sys, os, Image

__version__ = 0.02


def create_thumb(input_folder, file_full_path, file_name, thumb_height):
    """ creates a thumb of a pic by a given height with original aspectratio
        Paramters:
            inputfolder = just the path to the file
            file_full_path = path/to/file.ext
            file_name = just the filename, no path. no extension
            thumb_height =  (in pixel) used to calculate new_w by aspectratio of inputpic
    """

    im = Image.open(file_full_path)
    original_width, original_height = im.size
    thumb_width = thumb_height * original_width / original_height
    im.thumbnail((thumb_width, thumb_height), Image.ANTIALIAS)
    im.save(input_folder + file_name+ "-thumb.jpg", "JPEG")

    print "Thumb created:"
    print "--> " + input_folder + file_name + "-thumb.jpg"
    print " "

def every_image_in_folder(input_folder):
    """ walks through a given folder and calls create_thumb() for every image
        Paramters:
            inputfolder = folder with original pics
    """
    for this_file in os.listdir(input_folder):
        file_name = os.path.splitext(this_file)[0]
        file_full_path = os.path.join(input_folder , this_file)

        create_thumb(input_folder, file_full_path, file_name, thumb_height=100)

def main():
    pic_search_path="/home/sabba/in/pic/"

    every_image_in_folder(pic_search_path)

if __name__ == '__main__':
    main()


Mir gefallen die vielen Parameter für "create_thumb()" nicht.
Ich denke ich werde als Parameter nur den vollen Dateipfad und eben "thumb_height" übergeben.
Dann wird der Pfad halt in der Funktion aufgedröselt.
Dadurch kann ich dann auch den docstring kürzen.

Als Nächstes werde ich mich mal an die Parameter machen, die beim Aufruf über die Kommandozeile mitgegeben werden.

cu Sebastian
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Dienstag 8. Februar 2005, 14:01

Mr_Snede hat geschrieben:Nicht wirklich, ich habe die Sicht eines Studenten auf die Tageszeit:
Der Tag ist erst rum wenn ich ins Bett gehe :-)

Richtige Einstellung! *g*

Mr_Snede hat geschrieben:

Code: Alles auswählen

__version__ = 0.02

Ich täts ja so machen:

Code: Alles auswählen

__version__ = "0.0.2"
__versiontuple__ = tuple(__version__.split('.'))

So kann man dann später auch nach Major oder Minor Versionen abfragen.

Mr_Snede hat geschrieben:Mir gefallen die vielen Parameter für "create_thumb()" nicht.
Ich denke ich werde als Parameter nur den vollen Dateipfad und eben "thumb_height" übergeben.
Dann wird der Pfad halt in der Funktion aufgedröselt.
Dadurch kann ich dann auch den docstring kürzen.

Da du kannst auch eine Tupel übergeben, dann hast du nicht so viele Argumente, aber das bringt wohl auch nicht so genial viel.

Mr_Snede hat geschrieben:Als Nächstes werde ich mich mal an die Parameter machen, die beim Aufruf über die Kommandozeile mitgegeben werden.

Ich würde zu optparse/Optik raten, da könnte ich dir auch beispiele liefern und das Ding kann echt viel und ist recht einfach zu nutzen.
My god, it's full of CARs! | Leonidasvoice vs Modvoice
mawe
Python-Forum Veteran
Beiträge: 1209
Registriert: Montag 29. September 2003, 17:18
Wohnort: Purkersdorf (bei Wien [Austria])

Beitragvon mawe » Dienstag 8. Februar 2005, 14:10

Hi Leonidas!

Leonidas hat geschrieben:So kann man dann später auch nach Major oder Minor Versionen abfragen.

Könntest Du das bitte kurz genauer erklären. Das ist ziemlich neu für mich (was heisst ziemlich ... ich hab keinen Schimmer davon :D).

Gruß, mawe
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Dienstag 8. Februar 2005, 14:48

Ist jetzt nicht wirklich Python spezifisch, aber für das Parsen von Versionsinformationen ganz nett :)

Nun ja, so nutze ich sowas in der Richtung für WhatsOnAir:

Code: Alles auswählen

__version__ = "2.4.26"
__versiontuple__ = tuple(__version__.split('.'))

if __versiontuple__[1] % 2 != 0:
    debug = True
else:
    debug = False


Also, erstmal erstelle ich aus dem String eine Tupel. Dann gucke ich ub die zweite Nummer gerade ist, und wenn nicht, dann ist dies eine "unstable" Version, je nach dem kann ich dann diverse Sachen machen. Es ist hier genauso wie mit dem Linux Kernel, bei dem gerade stable ist und ungerade development.

Ja, ist nicht so der Hammer, ich weiß.
My god, it's full of CARs! | Leonidasvoice vs Modvoice
Benutzeravatar
Dookie
Python-Forum Veteran
Beiträge: 2010
Registriert: Freitag 11. Oktober 2002, 18:00
Wohnort: Salzburg
Kontaktdaten:

Beitragvon Dookie » Dienstag 8. Februar 2005, 15:14

Hi,

da könnte man doch auch schön eine Versionsklasse machen.

Code: Alles auswählen

class Version(tuple):
    """
        Class for versioninformation, representet by a tuple holding
        major-, minor- and subversion.
        To create an versioninstance use Version(0,1,0) or Version("0.1.0")
        The elements can also be called by the properties major, minor and sub
    """

    def major(self):
        return self[0]
    major = property(major)

    def minor(self):
        return self[1]
    minor = property(minor)

    def sub(self):
        return self[2]
    sub = property(sub)

    def __new__(cls, major, minor=0, sub=0):
        if isinstance(major, basestring):
            major, minor, sub = [int(x) for x in (major.split(".")+[0,0,0])[:3]]
        return super(Version, cls).__new__(cls, (major, minor, sub))

    def __repr__(self):
        return "Version(%i, %i, %i)" % self

    def __str__(self):
        return "%i.%i.%i" % self

    def __add__(self, other):
        major, minor, sub = [a+b for a, b in zip(self, other)]
        return type(self)(major, minor, sub)


Gruß

Dookie
Edit: fehler bei __str__ behoben.
Zuletzt geändert von Dookie am Dienstag 8. Februar 2005, 15:49, insgesamt 1-mal geändert.

Code: Alles auswählen

#!/usr/bin/env python
import this
mawe
Python-Forum Veteran
Beiträge: 1209
Registriert: Montag 29. September 2003, 17:18
Wohnort: Purkersdorf (bei Wien [Austria])

Beitragvon mawe » Dienstag 8. Februar 2005, 15:14

Hi Leonidas!

Find ich gar nicht so schlecht. Vielleicht klau ich Dir diese Idee :D
Sieht auf jeden Fall sehr professionell aus (und etwas Professionalität, wenn auch nur oberflächliche, kann meinen Scripts nicht schaden :wink:).

Danke für die Erklärung!

@Dookie: Das ist mir wiederum zu professionell :D

Gruß, mawe
Benutzeravatar
Leonidas
Administrator
Beiträge: 16023
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Beitragvon Leonidas » Dienstag 8. Februar 2005, 16:00

Naja, Dookie, ich finde meine Lösung irgendwie "einfacher". Deine ist ja schon nahe am Nonplusultra :)
My god, it's full of CARs! | Leonidasvoice vs Modvoice

Wer ist online?

Mitglieder in diesem Forum: 0 Mitglieder