@preklov: `os.path` braucht man nicht importieren, das `os`-Modul importiert das passende `path`-Modul wenn es das erste mal importiert wird.
Bei `TestNumbering` ist der reguläre Ausdruck ziemlich aufwändig. Zum Teil weil die falsche Funktion verwendet wird: `match()` statt `search()`. Ein ``re.search(r'[0-9]', values)`` würde reichen. Oder ohne `re`: ``any(c.isdecimal() for c in values)``.
Warum bindest Du in den beiden `__init__`-Methoden eigentlich `values` an einen einbuchstabigen Namen? Das macht nicht wirklich Sinn.
Die Verwendung des Begriffs Unicode finde ich etwas problematisch, denn zumindest unter Linux gibt es keine Unicode-Dateinamen — das sind beliebige Bytefolgen wobei '/' und Nullbytes generell eine bestimmte Bedeutung haben und ansonsten noch von den Dateisystemen Einschränkungen gemacht werden können. An der Stelle gruselts mich gerade was Python 3 mit den Kommandozeilenargumenten macht. Das sind auch Bytes und Python 3 scheint die einfach in irgendeiner geratenen Kodierung zu dekodieren. Damit wäre es zum Beispiel nicht möglich Dateinamen an ein Python-Programm zu übergeben die nicht dieser geratenen Kodierung entsprechen, aber durchaus gültige Dateinamen sind. WTF!?
Beim `SetStart` ist die Annahme mit der 1000 vielleicht etwas gefährlich. Das könnte potentiell durchaus ein gültiger Wert sein.
``\`` zur Zeilenfortsetzung versuche ich zu vermeiden. Danach darf kein Zeichen mehr kommen, auch kein „whitespace”. Das kann beispielsweise zu Problemen führen wenn man eine Datei mit Windows-Zeilenenden versucht unter Unix/Linux auszuführen. Dann bekommt man einen Syntaxfehler, den man in den meisten Editoren überhaupt nicht nachvollziehen kann wenn man nicht schon vorher weiss wo das Problem liegt. Man kann die fast immer vermeiden wenn man zusätzliche Klammern verwendet. Manchmal auch durch umformulieren der Bedingung. Das mindestens eine Option gebraucht wird kann man Beispielsweise mit `any()` formulieren:
Code: Alles auswählen
if not any([args.text, args.cut, args.ascii, args.sernum, args.substold]):
An zwei Stellen verwendest Du das ``\`` auch unnötigerweise weil an den Stellen noch Klammern ”offen” sind, der Compiler also weis, dass die Zeilen noch nicht zuende sind.
Der Rückgabecode, der per `sys.exit()` an das aufrufende Programm zurück gegeben wird, sollte nicht 0 sein wenn es ein Problem gab. Denn 0 bedeutet per Konvention, dass alles in Ordnung war und das Programm fehlerfrei durchgelaufen ist.
Der Test bei `args.ascii` ob die anderen Argumente nur ASCII-Zeichen enthalten muss IMHO nicht sein. Wenn die Transliteration nach den ganzen anderen Operationen stattfindet, würden diese Zeichen ja auch ersetzt.
Bei den Argumenten sollte `path` eigentlich `paths` heissen und der Hilfetext, dass hier Wildcards enthalten sein können ist falsch. Das Programm kann mit Wildcards doch überhaupt nichts anfangen.
Für das Übersetzen von Unicode nach ASCII könntest Du mal dieses Modul anschauen:
Unidecode.
In `get_names()` könnte man `collections.defaultdict` gut verwenden. Und man sollte als Werte vielleicht Mengen (`set`) statt Listen verwenden, sonst kann das bei vielen Dateien durchaus schon ziemlich zäh werden. Beim drüberfliegen über den restlichen Quelltext scheint das generell ein Muster zu sein, dass Du Listen verwendest wo man von den Operationen eigentlich Mengen bräuchte.
Insgesamt hat es eine Weile gedauert bis ich die Funktion verstanden habe, weil der DocString nicht wirklich passt. Die ausgewählten Namen von den verbleibenden Namen trennen, da fragt man sich welche von den Namen in `fileList` die ausgewählten sind, und kommt nicht so schnell darauf, dass das *alle* sind und das die „verbleibenden” innerhalb der Funktion noch aus dem Dateisystem gelesen werden. Der Name `restNames` hilft auch überhaupt nicht, denn `rest` ist hier wohl deutsch, hat aber auch im Englischen eine Bedeutung die hier so gar nicht passt. `restNames` würde ich auch nicht am Anfang an einen Namen binden, sondern erst dann, wenn man auch wirklich anfängt damit zu arbeiten. Das verringert die Gefahr von nicht benötigten Quellextzeilen. Wenn man später nämlich feststellt, dass man einen Namen irgendwo weiter unten in einer Funktion *direkt* an den endgültigen Wert binden kann, vergisst man gerne mal die dann unnötige „Initialisierung” weiter oben. Die Funktion könnte dann so aussehen:
Code: Alles auswählen
def get_names(filenames):
"""Get the filenames of the other files in the directories of given paths.
This is required to check for duplicate file names with modified names
in a directory. So all names are included, files as well as directories.
"""
dir2selected_filenames = defaultdict(set)
for fileName in filenames:
dirname, name = os.path.split(fileName)
dir2selected_filenames['.' if dirname == '' else dirname].add(name)
dir2other_filenames = dict(
(dirname, set(os.listdir(dir)) - names)
for dirname, names in dir2selected_filenames.items()
)
return (dir2selected_filenames, dir2other_filenames)
Den Ansatz finde ich aber aus zwei Gründen nicht gut. Zum einen ganz grundsätzlich weil so ein Test zu dem die Daten dann verwendet werden von der Wirklichkeit überholt werden kann, denn Verzeichnisinhalte sind ja nicht zwingend statisch. Zum anderen haben wir hier zwei parallele Datenstrukturen die herum gereicht werden. Da fängt OOP an sinnvoll zu werden. Oder zumindest eine andere Strukturierung der Daten.
Aber wie gesagt, finde ich diesen Prüfansatz schon nicht gut. Dass man innerhalb der Namen prüft die umbenannt werden sollen ist okay, aber mit den anderen sollte man IMHO entweder gar nicht, oder aber erst unmittelbar vor dem Umbenennen etwas anstellen. Oder, so wie das viele Werkzeuge dieser Art tun, eine Option bieten, die vor dem umbenennen versucht den neuen Namen in etwas mit einer Endung wie '~' oder '.bak' umzubenennen, um Datenverluste zu vermeiden. Letztendlich ist umbenennen immer gefährlich.
Man muss nicht um jeden Preis das verketten von Zeichenketten mit ``+`` vermeiden. In `modify_name()` sind zwei Codestellen die einfacher werden wenn man ``+`` statt `format()` oder gar `join()` mit einer Liste mit zwei Elementen verwendet. Beim Rückgabewert ist `re` ein bisschen überdimensioniert. Die letzten beiden Zeilen könnte man durch das wesentlich simplere ``return baseName.rstrip(' .') + ext`` ersetzen.
Der Kommentar dort ist übrigens eine Information die für *Benutzer* vielleicht interessant wäre. Es können ja durchaus Namen vorkommen die so aussehen: „Something Wonderful 2nd Pt..ogg” und da wäre ich überascht wenn mir beim Nummerieren plötzlich der Punkt von 'Pt.' „geklaut” wird.
In `show_diffs()` sieht man wieder lauter parallele Datenstrukturen. Ausserdem fällt dort auf, dass Du immer Teilübersetzungen zusammensetzt. Das sollte man nicht machen, denn es kann durchaus sein, dass die damit von Dir vorgegebene Reihenfolge nicht in jeder Sprache zu grammatikalisch richtigen Übersetzungen führt. Es kann zum Beispiel sein, dass bei Anzahlen 'x Dinge' in einer anderen Sprache erst das Nomen und *dann* die Anzahl kommt die sich auf das Nomen bezieht. Für Variation bei verschiedenen Anzahlen gibt es ausserdem spezielle Unterstützung in `gettext`, denn das man nur Einzahl bei x=1 und Mehrzahl unterscheidet ist auch nicht in jeder Sprache gegeben. Es gibt auch welche wo keine, ein, zwei, drei, und viele Dinge sich grammatikalisch unterscheiden. Oder welche bei denen sich die Grammatik ändert wenn die Anzahl mit bestimmten Ziffern endet und so weiter. Der Abschnitt
Additional functions for plural forms aus dem „GNU gettext”-Handbuch hat Beispiele. Und die Sprachen sind auch nicht alle „exotisch”, es sei denn man sieht unsere polnischen Nachbarn schon als exotisch an.
Wieder parallele Datenstrukturen in `process_names()` und hier wären Mengen statt Listen wieder praktisch.
`msg1`, `msg2`, und `msg3` sind nichtssagende Namen und wie gesagt: Man sollte keine Teilübersetzungen machen.
`count` und das Zusammensetzen von einem fortlaufenden Zähler mit Prä- und Postfix ist IMHO zu undurchsichtig umgesetzt. Das könnte man schön in eine Generatorfunktion kaspeln, die fortlaufende Zeichenketten mit Nummern generiert.
Die Meldung mit `msg2` ist nicht zufriedenstellend. Für jeden Dateinamen den das betrifft, wird eine Meldung ausgegeben, dass mindestens ein Dateiname im Verzeichnis zu kurz wird. An der Stelle möchte man dann doch als Benutzer gerne wissen *welchen* Namen das betrifft. Die Information existiert an der Stelle doch sogar, man müsste sie nur ausgeben.
``continue`` mag ich persönlich nicht so gerne, weil man da in einer tieferen Ebene einen Sprung an den Schleifenanfang ”versteckt”.
Was der Name `nocorr` in `rename_files()` bedeuten soll, habe ich auch nach längerem Überlegen nicht herausgefunden. Mir ist die Bedeutung des Objekts klar, aber die hätte ich nicht vom Namen ablesen können. Und wieder parallele Datenstrukturen.
Den Fehler hätte man vielleicht auch Speichern sollen, damit der Benutzer erfahren kann warum das umbenennen nicht funktioniert hat.
`asciiErrorNames` in der `main()`-Funktion wird als Argument an andere Funktionen übergeben um dort gefüllt zu werden. Da hätte man vielleicht besser mit Rückgabewerten gearbeitet, dann wäre es deutlicher was da passiert.
Die „list comprehension” als ``if``-Bedingung ist komisch. Ein ``if any(newNames.values()):`` ist kürzer, verständlicher, und effizienter wohl auch.
Man kann das Programm gar nicht ohne echten Benutzer laufen lassen, also zum Beispiel von einem Cronjob aus. Die Nachfrage würde ich durch eine Kommandozeilenoption ersetzen die man angeben muss wenn tatsächlich umbenannt werden soll. Man kann auch zusätzlich eine Option für einen interaktiven Modus hinzufügen, bei dem dann per `input()` nachgefragt wird.