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

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()
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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 (former) Modvoice
BlackJack

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

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
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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 (former) Modvoice
Dookie
Python-Forum Veteran
Beiträge: 2010
Registriert: Freitag 11. Oktober 2002, 18:00
Wohnort: Salzburg
Kontaktdaten:

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]#!/usr/bin/env python
import this[/code]
Benutzeravatar
Mr_Snede
User
Beiträge: 387
Registriert: Sonntag 8. Februar 2004, 16:02
Wohnort: D-Dorf, Bo

@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
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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

@ 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
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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 (former) Modvoice
mawe
Python-Forum Veteran
Beiträge: 1209
Registriert: Montag 29. September 2003, 17:18
Wohnort: Purkersdorf (bei Wien [Austria])

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
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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 (former) Modvoice
Dookie
Python-Forum Veteran
Beiträge: 2010
Registriert: Freitag 11. Oktober 2002, 18:00
Wohnort: Salzburg
Kontaktdaten:

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

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
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

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 (former) Modvoice
Dookie
Python-Forum Veteran
Beiträge: 2010
Registriert: Freitag 11. Oktober 2002, 18:00
Wohnort: Salzburg
Kontaktdaten:

Hi Leonidas,

ich poste gleich die Nonplusultraversionklasse in den Codesnippets.
Das ganze ist natürlich für ein eigenes Version-Modul gedacht und würde dann einheitliche Versionsverarbeitung erlauben.


Gruß

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

@ leonidas
über optparse bin ich in den letzten Tagen auch gestolpert (hier im Forum) aber erst ist getopt dran.

@Dookie
Du nimmst mir meinen Vorschlag vorweg: Diese Klasse gehört in die Codesnippets.
Aber für mein Skript ist das dann doch etwas zu wuchtig[1].
Ansonsten Danke.

cu Sebastian

[1] wenn ich mal groß bin und mit großen Programmen die Weltherrschaft an mich reiße werde ich das nur mit deiner Nonplusultraversionklasse machen :-)
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Dookie hat geschrieben:ich poste gleich die Nonplusultraversionklasse in den Codesnippets.
Das ganze ist natürlich für ein eigenes Version-Modul gedacht und würde dann einheitliche Versionsverarbeitung erlauben.
Nett. Wie wärs mit Verwalung von CVS und SVN $Id$ Strings?
Mr_Snede hat geschrieben:@ leonidas
über optparse bin ich in den letzten Tagen auch gestolpert (hier im Forum) aber erst ist getopt dran.
Ich würde gleich mit getopt anfangen, ich nutze es seit Python 2.3 als es in die stdlib kan, und habe sowas vorher nie gemacht. Vielleicht interessiert dich ja noch getargs ;)
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
Benutzeravatar
Mr_Snede
User
Beiträge: 387
Registriert: Sonntag 8. Februar 2004, 16:02
Wohnort: D-Dorf, Bo

@ Leonidas
Ich würde gleich mit getopt anfangen ...
Das habe ich doch auch vor. Oder hast du dich nur vertippt und optparse gemeint?

Ich nehme getopt einfach weil in meiner Hardware Doku schon die Lesezeichen drinstecken.
Optparse werde ich aber auch noch ausprobieren.
Aber erst, wenn ich mir 'ne GUI zu dem Skript gebaut habe.
Oder auch mehrere, mit disem Script wollte ich mal ein paar GUI-Toolkits ausprobieren.

cu Sebastian
Leonidas
Python-Forum Veteran
Beiträge: 16025
Registriert: Freitag 20. Juni 2003, 16:30
Kontaktdaten:

Mr_Snede hat geschrieben:@ Leonidas
Ich würde gleich mit getopt anfangen ...
Das habe ich doch auch vor. Oder hast du dich nur vertippt und optparse gemeint?
Notiz für mich: Erst denken dann schreiben. Ja ich habe optparse zum anfangen gemeint.
Mr_Snede hat geschrieben:Ich nehme getopt einfach weil in meiner Hardware Doku schon die Lesezeichen drinstecken.
Optparse werde ich aber auch noch ausprobieren.
Aber erst, wenn ich mir 'ne GUI zu dem Skript gebaut habe.
Oder auch mehrere, mit disem Script wollte ich mal ein paar GUI-Toolkits ausprobieren.
Was die GUI Toolkits angeht kann ich dir auch ein paar Tipps geben (kann dir fast jeder in diesem Forum *g*).
My god, it's full of CARs! | Leonidasvoice vs (former) Modvoice
Antworten