Optimisation d'un programme de tri de musique

Version imprimable

Bonjour, j'ai écrit un programme qui tri automatiquement la musique dans des dossiers en fonction du nom de l'artiste et de l'album, étant mon premier vrai programme, j'aimerais savoir si mon code est optimisé ou s'il y a des erreurs qui passent inaperçues.

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
import mutagen
import os
import glob
 
#List all mp3 files in directory
def list_all_mp3():
    mp3_list = glob.iglob('*.mp3', recursive=False)
 
    return mp3_list
 
#Read the tag of the mp3 file
def get_mp3tag(file_name):
    audio = mutagen.File(file_name)
    album = str(audio['TALB'].text).strip("['']")
    artist = str(audio['TPE1'].text).strip("['']")
    return [artist, album]
 
#Verify if folder exist else create the folder
def create_folder(artist, album):
    if os.path.isdir(artist) is False:
        os.mkdir(artist)
        os.mkdir(artist+"/"+ album)
    elif os.path.isdir(artist+"/"+ album) is False:
        os.mkdir(artist+"/"+ album)
    else:
        print("The directory has been already created")
 
#Move the music to the folder
def move_mp3file(file_name, artist, album):
    if os.path.exists(artist + "/"+ album + "/" + file_name) is False:
        os.rename(file_name, artist + "/"+ album + "/" + file_name)
    else:
        print("This music is already in the directory")
 
def main():
    mp3_list = list_all_mp3()
 
    for mp3 in mp3_list:
        artist_album = get_mp3tag(mp3)
        artist = str(artist_album[0]).rstrip().translate({ord(c): None for c in '?!@#$:'})
        album = str(artist_album[1]).rstrip().translate({ord(c): None for c in '?!@#$:'})
        create_folder(artist, album)
        move_mp3file(mp3, artist, album)
 
if __name__ == "__main__":
    main()

Merci, d'avance

21/10/2018, 16h54
lg_53
Je n'ai pas éxécuter votre code mais pour un 1er vrai programme ca n'a pas l'air trop mal.

Je commente quelques points :
1)
Code:

1 2 album = str(audio['TALB'].text).strip("['']") artist = str(audio['TPE1'].text).strip("['']")
c'est bizarre votre syntaxe là. Ceci laisse penser que audio['TALB'].text est une liste, que vous transformer en string et vous enlevez ensuite les crochets à la main. Si c'est une liste d'un seul élément, peut-être tout simplement :

Code:

artist = (audio['TPE1'].text)[0]

(les parenthèses ne sont peut etre pas obligatoire je ne sais plus)

2) Pour gérer les chemins, au lieu de faire une concaténation avec "/", on utilisera plutot os.path.join. Car le caractère de séparateur de fichier n'est pas le même sous windows que sur linux. os.path.join gère ca tout seul.

3) Pour la fonction create_folder tu peux certainement t'appuyer un peu plus sur les choses qui existe en python. Je pense notamment à os.makedirs qui est capable de créer tout une série de dossier imbriqué jusqu'au dossier voulu. Et li possède même un argument optionnel [C]exist_ok[C] qui te permet de choisir si tu veux déclencher une erreur ou non si le répertoire final existe déjà.

4)

Code:

.rstrip().translate({ord(c): None for c in '?!@#$:'})

apparait 2 fois. Et en plus c'est pas super évident de comprendre ce que ca fait de prime abord. Donc hésite pas à le mettre dans une petite fonction , et à donner à cette fonction un nom explicite du style remove_special_character par exemple.

5) Conceptuellement tu fabrique le dossier et après tu déplace le fichier dedans
Code:

1 2 create_folder(artist, album) move_mp3file(mp3, artist, album)
Tu pourrais très bien appeler create_folder dans la fonction move_mp3file. Ainsi l'utilisateur de la fonction move_mp3file n'a pas à se soucier de savoir si le dossier existe bien. La fonction gère cela toute seule.

Merci beaucoup pour tes conseils, voici mon code modifié:

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
import mutagen
import os
import glob
 
#List all mp3 files in directory
def list_all_mp3():
    mp3_list = glob.iglob('*.mp3', recursive=False)
 
    return mp3_list
 
#Read the tag of the mp3 file
def get_mp3tag(file_name):
    audio = mutagen.File(file_name)
    album = (audio['TALB'].text)[0]
    artist = (audio['TPE1'].text)[0]
    return [artist, album]
 
#Verify if folder exist else create the folder
def create_folder(artist, album):
    try:
        os.makedirs(os.path.join(artist, album), exist_ok=False)
    except OSError:
        print("The directory has been already created")
 
#Move the music to the folder
def move_mp3file(file_name, artist, album):
    create_folder(artist, album)
    try:
        os.rename(file_name, os.path.join(artist, album, file_name))
    except OSError:
        print("This music is already in the directory")
 
#Remove reserved characters
def remove_special_character(file):
    file = str(file.rstrip().translate({ord(c): None for c in '/\?%*:|"<>.'}))
    return file
 
def main():
    mp3_list = list_all_mp3()
 
    for mp3 in mp3_list:
        artist_album = get_mp3tag(mp3)
        artist = remove_special_character(artist_album[0])
        album = remove_special_character(artist_album[1])
        move_mp3file(mp3, artist, album)
 
if __name__ == "__main__":
    main()

22/10/2018, 08h46
lg_53
Tu n'utilises pas la structure de liste ici

Code:

return [artist, album]

donc tu peux retourner un tuple

Code:

return artist, album

et lorsque tu fais appel à cette fonction tu peux directement dépaqueter (unpacking) le tuple :
Code:

1 2 3 artist_album = get_mp3tag(mp3) artist = remove_special_character(artist_album[0]) album = remove_special_character(artist_album[1])
devient :
Code:

1 2 3 artist,album = get_mp3tag(mp3) artist = remove_special_character(artist) album = remove_special_character(album)
Tu peux également raccourcir l'écriture de certaines fonctions :
Code:

1 2 3 def fct(arg): a= 'blabla' return a
peut s'écrire aussi
Code:

1 2 def fct(arg): return 'blabla' ### directement
ou encore si tu connais les lambdas fonction

Code:

fct = lambda arg : 'blabla'

Dernier point :

Code:

def remove_special_character(file):

tu ne supprime pas les charactères spéciaux présents dans le fichier. Tu les supprime du nom de fichier. Nommé le paramètre filename serait peut etre plus judicieux.
Et .translate retourne un str il me semble donc pas besoin de caster en str.

Encore merci, je ne savais pas que je pouvais utiliser les tuples de la sorte, par contre j'ai du mal a comprendre l'utilité de la fonction lambda.
De plus j'hésite a raccourcir cette partie de peur de perdre en lisibilité:

Code:

1
2
3
4
5
    for mp3 in mp3_list:
        artist,album = get_mp3tag(mp3)
        artist = remove_special_character(artist)
        album = remove_special_character(album)
        move_mp3file(mp3, artist, album)

par:

Code:

1
2
3
    for mp3 in mp3_list:
        artist,album = get_mp3tag(mp3)
        move_mp3file(mp3, remove_special_character(artist), remove_special_character(album))

Le Code modifié:

Code:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
import mutagen
import os
import glob
 
#List all mp3 files in directory
def list_all_mp3():
    return glob.iglob('*.mp3', recursive=False)
 
#Read the tag of the mp3 file
def get_mp3tag(file_name):
    audio = mutagen.File(file_name)
    album = (audio['TALB'].text)[0]
    artist = (audio['TPE1'].text)[0]
    return artist, album
 
#Verify if folder exist else create the folder
def create_folder(artist, album):
    try:
        os.makedirs(os.path.join(artist, album), exist_ok=False)
    except OSError:
        print("The directory has been already created")
 
#Move the music to the folder
def move_mp3file(file_name, artist, album):
    create_folder(artist, album)
    try:
        os.rename(file_name, os.path.join(artist, album, file_name))
    except OSError:
        print("This music is already in the directory")
 
#Remove reserved characters
def remove_special_character(file_name):
    return file_name.rstrip().translate({ord(c): None for c in '/\?%*:|"<>.'})
 
def main():
    mp3_list = list_all_mp3()
 
    for mp3 in mp3_list:
        artist,album = get_mp3tag(mp3)
        artist = remove_special_character(artist)
        album = remove_special_character(album)
        move_mp3file(mp3, artist, album)
 
if __name__ == "__main__":
    main()

22/10/2018, 16h11
lg_53
Citation:

Envoyé par Flowrey

Encore merci, je ne savais pas que je pouvais utiliser les tuples de la sorte, par contre j'ai du mal a comprendre l'utilité de la fonction lambda.

le mot clé lambda remplace def, et permet de définir des fonctions à la volée. Il y a de la doc là dessus sur la toile, tu trouveras de quoi comprendre :
https://python.developpez.com/cours/..._functions.php
https://www.w3schools.com/python/python_lambda.asp

Citation:

Envoyé par Flowrey

De plus j'hésite a raccourcir cette partie de peur de perdre en lisibilité:

Ca c'est toi qui voit, c'est un argument recevable. Pour moi tant que ca tient sur une ligne ca va, mais chacun a sa limite de lisibilité.
La questions se pose là aussi :
Code:

1 2 3 4 5 def get_mp3tag(file_name): audio = mutagen.File(file_name) album = (audio['TALB'].text)[0] artist = (audio['TPE1'].text)[0] return [artist, album]
à raccourcir ou pas en :
Code:

1 2 3 def get_mp3tag(file_name): audio = mutagen.File(file_name) return (audio['TPE1'].text)[0] , (audio['TALB'].text)[0]