Optimisation d'un programme de tri de musique

**Flowrey** · 21/10/2018, 11h17

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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

**lg_53** · 21/10/2018, 16h54

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

.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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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.

**Flowrey** · 21/10/2018, 17h56

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

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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()

**lg_53** · 22/10/2018, 08h46

Tu n'utilises pas la structure de liste ici

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

    return [artist, album]

donc tu peux retourner un tuple

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

    return artist, album

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

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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

devient :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
3
def fct(arg):
    a= 'blabla'
    return a

peut s'écrire aussi

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

1
2
def fct(arg):
    return 'blabla' ### directement

ou encore si tu connais les lambdas fonction

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

fct = lambda arg : 'blabla'

Dernier point :

Code :

Sélectionner tout - Visualiser dans une fenêtre à part

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.

**Flowrey** · 22/10/2018, 14h21

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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()

**lg_53** · 22/10/2018, 16h11

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

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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 :

Sélectionner tout - Visualiser dans une fenêtre à part

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

Optimisation d'un programme de tri de musique [Python 3.X]

Python

Discussions similaires

Partager

Partager