Critique de code

**Supernul** · 23/02/2012, 15h22

Bonjour à tous,

je débute Python et j'aimerais votre avis sur mon premier script... donc lâchez-vous :

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
import os
import glob
import numpy as np
Lst1=os.listdir('C:/Users/gaetan.palka/Desktop/DATA_RM') #liste pour les codes des volontaires et des cartes
Lst2=glob.glob('C:/Users/gaetan.palka/Desktop/DATA_RM/*K*.txt') #liste les calculs
XbarYbarDisp=np.zeros((len(Lst2),5))  #matrice qui contiendra les coordonnées du barycentre pondéré et la dispersion
Lst1_1C=np.reshape(Lst1,(len(Lst1),1)) #mise sur une colonne de la liste des fichiers à traiter
taille=np.zeros((len(Lst2),1))
for i in range(len(Lst2)): #pour chaque fichier listé
        #déclaration des variables
        Cont_Lst2=np.loadtxt(Lst2[i]) #chargement dans Cont_lst2 du contenu de Lst2
        Coordpond=np.zeros((len(Cont_Lst2),2))
        DistEuc=np.zeros((len(Cont_Lst2),1))
        DistEucPond=np.zeros((len(Cont_Lst2),1))
        Var=np.zeros((1,1))
        Xbar=np.zeros((1,1))
        Ybar=np.zeros((1,1))
        SumFix=np.zeros((1,1))
        #calcul de la durée de fixation totale, du barycentre non pondéré, du nombre de fixations
        SumFix=np.sum(Cont_Lst2[:,4])
        Xbar=np.mean(Cont_Lst2[:,1])
        Ybar=np.mean(Cont_Lst2[:,2]*-1)
        for j in range(len(Cont_Lst2)): #pour chaque ligne de fichier : calcul des coordonnées pondérées, du barycentre pondéré, du vecteur non pondéré puis pondéré, variance
                Coordpond[j,0]=Cont_Lst2[j,1]*Cont_Lst2[j,4]
                Coordpond[j,1]=Cont_Lst2[j,2]*Cont_Lst2[j,4]*-1
                DistEuc[j,0]=(Cont_Lst2[j,1]-Xbar)**2+(-1*Cont_Lst2[j,2]-Ybar)**2
                DistEucPond[j,0]=Cont_Lst2[j,4]*DistEuc[j]
                Var=np.sum(DistEucPond[:,0])/SumFix
        XbarYbarDisp[i,0]=np.sum(Coordpond[:,0])/SumFix
        XbarYbarDisp[i,1]=np.sum(Coordpond[:,1])/SumFix
        XbarYbarDisp[i,2]=np.sqrt(Var)
        XbarYbarDisp[i,3]=Xbar
        XbarYbarDisp[i,4]=Ybar
Lst1_2C = []
for el in Lst1_1C: # mise sur deux colonnes de la liste
        Lst1_2C.append(el[0][:-4].split('_',))
F=np.concatenate((Lst1_2C,XbarYbarDisp),axis=1)
resultats=open("ResBarDis.txt",'w') #mise en mémoire du fichier de sortie
resultats.write('\n'.join(' '.join(l) for l in F)) #écriture du fichier de sortie
resultats.close() #retrait de la mémoire du fichier de sortie

Les 6 dernière lignes ne sont pas de moi mais d'une aide généreuse de ce forum.

Merci d'avance.

**fred1599** · 23/02/2012, 15h26

Intérêt de ton code?

Sous quel format d'entrée sont les fichiers textes (un exemple)

**Supernul** · 23/02/2012, 15h56

les fichiers textes en entrée ont ce format :

Code :

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

1
2
3
4
5
6
7
8
9
10
11
12
13
1	-1.03	1.59	0.02	0.20
2	-4.56	-0.31	0.25	0.10
3	-10.90	-0.73	0.38	0.27
4	-9.21	-7.17	0.68	0.37
5	-11.50	-9.47	1.08	0.33
6	-14.00	-3.89	1.45	0.30
7	10.60	-10.70	1.82	0.20
8	9.56	-12.90	2.05	0.57
9	15.50	-12.00	2.72	0.20
10	18.10	-12.40	2.95	0.17
11	-11.60	-1.11	3.28	0.20
12	-15.30	1.01	3.52	0.13
13	-15.70	-4.00	3.68	1.03

Accessoirement j'en ai 800, le nombre de ligne par fichier texte varie entre 8 et 50.
Les colonnes :
- identifiant
- X
- -Y
- moment de la mesure
- durée de la mesure

Le script me sert à calculer le barycentre des points, le barycentre pondéré et la dispersion des points pondérés (pondération en dernière colonne)

**ZZelle** · 23/02/2012, 21h40

Je ne sais pas si la critique du code doit porter sur le fond ou la forme.
Mais n'utilisant pas numpy, je parlerais surtout de la forme et de l'organisation du code

1) On respecte en python pour faciliter la lecture les conventions du pep8, càd:

Code :

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

1
2
3
XbarYbarDisp[i,0]=np.sum(Coordpond[:,0])/SumFix
# s'écrira plutôt
xbar_ybar_disp[i, 0] = np.sum(coordpond[:, 0]) / sum_fix

2) Ton code est très dense, trop de variables, des noms de variables incompréhensibles ...
Un moyen d'éviter ce genre de problème est de découper ton code en fonctions.
Par exemple, le contenu de ta 1ère boucle peut être mis dans une fonction:

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
 
def compute_barycenter(matrix):
     Coordpond=np.zeros((len(Cont_Lst2),2))
        DistEuc=np.zeros((len(Cont_Lst2),1))
        DistEucPond=np.zeros((len(Cont_Lst2),1))
        Var=np.zeros((1,1))
        Xbar=np.zeros((1,1))
        Ybar=np.zeros((1,1))
        SumFix=np.zeros((1,1))
        #calcul de la durée de fixation totale, du barycentre non pondéré, du nombre de fixations
        SumFix=np.sum(Cont_Lst2[:,4])
        Xbar=np.mean(Cont_Lst2[:,1])
        Ybar=np.mean(Cont_Lst2[:,2]*-1)
        for j in range(len(Cont_Lst2)): #pour chaque ligne de fichier : calcul des coordonnées pondérées, du barycentre pondéré, du vecteur non pondéré puis pondéré, variance
                Coordpond[j,0]=Cont_Lst2[j,1]*Cont_Lst2[j,4]
                Coordpond[j,1]=Cont_Lst2[j,2]*Cont_Lst2[j,4]*-1
                DistEuc[j,0]=(Cont_Lst2[j,1]-Xbar)**2+(-1*Cont_Lst2[j,2]-Ybar)**2
                DistEucPond[j,0]=Cont_Lst2[j,4]*DistEuc[j]
                Var=np.sum(DistEucPond[:,0])/SumFix
     return (np.sum(Coordpond[:,0])/SumFix,
               np.sum(Coordpond[:,1])/SumFix
               np.sqrt(Var),
               Xbar,
               Ybar)
 
 
barycenters = [] #remplace XbarYbarDisp
for matrix_file in matrix_files:
     matrix = np.loadtxt(matrix_file)
    barycenter = compute_barycenter(matrix)
    barycenters.append(barycenter)
...

3) Ca me semble louche que tu redéfinisses autant de variables pour les utiliser si peu de fois.
3-1)Et surtout pourquoi tu donnes des valeurs à des variables sans jamais les utiliser:

Code :

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

1
2
3
4
 
Xbar=np.zeros((1,1))
# code n'utilisant pas Xbar ... donc la ligne ci-dessus est inutile
Xbar=np.mean(Cont_Lst2[:,1])

Il en va de même pour Ybar, Var, SumFix

4) Je vois pas très bien l'intérêt de remplir une liste si c'est pour sommer son contenu après:

Code :

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

1
2
3
4
5
6
7
8
9
10
11
 
for j in range(len(Cont_Lst2)):
     # blah
     DistEucPond[j,0]=Cont_Lst2[j,4]*DistEuc[j]
Var=np.sum(DistEucPond[:,0])/SumFix
# s'écrira plutôt
sigma_var = 0
for i in Cont_Lst2:
     # blah
    sigma_var += Cont_Lst2[j,4]*DistEuc[j]
var = sigma_var / SumFix

Même question pour Coordpond ...

Du coup, j'arrive à un code du style (je dis pas qu'il marche, je connais pas numpy):
Remarque: ya vraiment besoin de numpy pour faire ce genre de calcul ? Vu qu'en fait tu fais des calculs sur une liste de points pondérés et non sur une matrice ou un tableau à 2 dimensions.

J'ai omis la partie concernant List1* mais je me demande l'utilité d'utiliser numpy.reshape et numpy.concatenate qui semble servir à de la mise en page

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
 
# liste des fichiers de calcul
matrix_files = glob.glob('C:/Users/gaetan.palka/Desktop/DATA_RM/*K*.txt')
 
def compute_barycenter(matrix):
    #calcul de la durée de fixation totale, du barycentre non pondéré, du nombre de fixations
    SumFix = np.sum(matrix[:, 4])
    Xbar = np.mean(matrix[:, 1])
    Ybar = np.mean(matrix[:, 2] * -1)
 
    sigma_var = 0
    sigma_coord0 = 0
    sigma_coord1 = 0 
    for j in range(len(matrix)):
        #pour chaque ligne de fichier : calcul des coordonnées pondérées, du barycentre pondéré, du vecteur non pondéré puis pondéré, variance
        sigma_coord0 += matrix[j, 1] * matrix[j, 4]
        sigma_coord0 += matrix[j, 2] * matrix[j, 4] * -1
        euclidian_dist = (matrix[j, 1] - Xbar) ** 2 + (-1 * matrix[j, 2] - Ybar) ** 2
        sigma_var += euclidian_dist * matrix[j, 4]
    return (sigma_coord0 / SumFix,
            sigma_coord1 / SumFix,
            np.sqrt(sigma_var / SumFix),
            Xbar,
            Ybar)
 
barycenters = []
for matrix_file in matrix_files:
    matrix = np.loadtxt(matrix_file)
    barycenter = compute_barycenter(matrix)
    barycenters.append(barycenter)

**Supernul** · 24/02/2012, 14h09

Merci ZZelle pour cette réponse.
Toute critique quelle qu'elle soit est la bienvenue (fond et/ou forme).

Comme je me lance seul dans Python, je fais avec ce que je trouve (je n'ai aucun bagage informatique pour ainsi dire).

1) Je prends note des conventions du pep8 qui sont très utiles.

2) Je commence à lire sur les fonctions et je vais m'y mettre car ça va m'alléger la tâche par la suite.

reshape et concatenate me serve en effet pour la mise en page/forcer le formatage car j'avais des problèmes de dimensions (utilisation de listes et de matrices). J'essaie une autre méthode pour le moment.

**BUENOO** · 24/02/2012, 14h27

Salut Supernul,

Si tu démarres tout seul je me permet de te partager certains liens que j'ai trouvé très utile concernant mon apprentissage qui est toujours en cours :
- http://www.biologeek.com/bonnes-prat...stuces-python/
- http://docs.python.org/py3k/library/
- http://docs.python.org/py3k/contents.html
- http://www.doughellmann.com/PyMOTW/

BUENOO.

Critique de code

Python

Discussions similaires

Partager

Partager