petit algo, pour avis

petit algo, pour avis - Python - Programmation

Marsh Posté le 09-10-2018 à 14:37:14    

Bonjour,
 
je me permets de vous soumettre un petit code python que j'ai écrit, afin de parser un répertoire de façon récursive, de récupérer seulement les fichiers dont l'extension est xml et hex ( la racine est la même) et d'extraire dans l'un le checksum en texte, et l'autre en binaire à une adresse donnée. Le but est de ressortir les fichiers qui ont des checksums différents.
 
Voici le résultat :
 

Code :
  1. import os
  2. import xml.etree.ElementTree as ET
  4. def r_parse_xml(e):
  5.     try:
  6.         tree = ET.parse( e['xml'])
  7.         root = tree.getroot()
  8.     except:
  9.         print("Echec de lecture de  " , e['xml'])
  10.     else:
  11.         '''
  12.         <fichier.configuration>
  13.             <BlocPlugAndPlay>
  14.  <pnp_fin label="M9012"/>
  15.  <pnp_idcan hexa="100"/>
  16.  <pnp_ring hexa="0"/>
  17.  <pnp_revision label="01"/>
  18.  <pnp_checksum hexa="ADC7"/>
  19.  <pnp_brin value="0"/>
  20.             </BlocPlugAndPlay>
  21. </fichier.configuration>
  22.         '''
  23.         for fin in root.iter("pnp_fin" ): #only once !
  24.             e['fin'] = fin.attrib['label']
  25.         for chk in root.iter("pnp_checksum" ): #only once !
  26.             e['chk_xml'] = chk.attrib['hexa'].rjust(4,'0')         
  27.     return e
  29. def r_parse_hex(e):
  30.     try:
  31.         f = open(e['hex'] , 'rb')
  32.     except:
  33.         print("Error opening " , e['hex'] )
  34.     else:
  35.         ret = f.read(0x400) # lire entete eeprom
  36.         e['chk_hex'] = ret[0x212:0x214][:-1].hex().upper()         
  37.     finally:
  38.         f.close()
  39.     return e
  41. def r_parse ( e , t ):
  42.     if t=="xml":
  43.         return r_parse_xml(e)
  44.     elif t=="hex":
  45.         return r_parse_hex(e)
  46.     return None
  48. def r_insert( r , t , b , n ):
  49.     for elem in r:
  50.         if elem['base'] == b:
  51.             elem[t]=n
  52.             r_parse( elem , t )
  53.             return elem
  55.     new_elem = { 'base' : '' , 'fin' : '' , 'xml' : '' , 'hex' : '' , 'chk_xml' : '' , 'chk_hex' : '' }
  56.     new_elem['base'] = b
  57.     new_elem[t] = n
  58.     r_parse( new_elem , t )
  59.     r.append(new_elem)
  60.     return new_elem   
  62. if __name__ == '__main__':
  63.     result = []   
  64.     for root,dirs,files in os.walk("." ):       
  65.         for name in files:
  66.             fname = os.path.join(root, name)         
  67.             base , ext = os.path.splitext( name)           
  68.             if ext==".hex" or ext==".xml":               
  69.                 r_insert( result , ext[1:] , base , fname )
  71.     for elem in result:
  72.         if elem['chk_xml']==elem['chk_hex']:
  73.             print("GOOD \t" + elem['fin'].rjust(16,' ') + "\t(chk=" + elem['chk_xml']+" )" )
  74.         else:
  75.             print("BAD  \t" + elem['fin'].rjust(16,' ') + "\t(chk_xml=" + elem['chk_xml']+",chk_hex="+elem['chk_hex']+" )" )


 
le code est bien entendu critiquable, je ne suis pas familier du python même si j'ai déjà écrit quelques programmes, j'essaie juste d'écrire à la "python" plutot qu'à la "C++" étant issu du C++.
 
Voici les choses que j'estime critiquables :
 - mauvaise gestion des exceptions. Bien que ca existe en C++, je ne les ai jamais vraiment utilisées (on peut s'en passer ) sauf que c'est un peu obligatoire en python. Je ne sais pas quoi mettre dans try, except, else, et finally ( voir fonction r_parse_hex et r_parse_xml )
 - j'ai un peu bidouillé pour formatter ma string venant du fichier xml correctement en hex : j'ai paddé avec des 0 pour être sur d'avoir 4 caractères ( e['chk_xml'] = chk.attrib['hexa'].rjust(4,'0')   )
 - j'ai encore plus bidouiller pour formater la string venant du fichier hex :  
   

Code :
  1. ret = f.read(0x400) # lire entete eeprom
  2.       e['chk_hex'] = ret[0x212:0x214][:-1].hex().upper()


             * je lis 1024 octets ( c'est normalement sur mais j'ai pas géré si inférieur )
             * mon checksum est à l'octet 0x212 + 0x213 mais à l'envers ( little endian ) , je l'inverse en rajoutant [:-1] puis transforme en string avec hex(), puis je passe en majuscule avec upper()
 - mon parcours de fichier est comme ca :  

Code :
  1. for root,dirs,files in os.walk("." ):       
  2.         for name in files:
  3.             fname = os.path.join(root, name)         
  4.             base , ext = os.path.splitext( name)           
  5.             if ext==".hex" or ext==".xml":               
  6.                 r_insert( result , ext[1:] , base , fname )


 
 mais j'aurais préféré un truc du genre :

Code :
  1. [  os.path.join(dp, f) for dp, dn, fn in os.walk(os.path.expanduser("." )) for f in fn]

 
sauf que je ne sais pas comment rajouter dans la ligne du code permettant de filtrer d'une part les hex/xml, et appeler une fonction d'autre part à chaque itération pour effectuer le traitement.
 
Bref, n'hésitez pas à faire vos commentaires si vous en avez envie, l'objectif pour moi est de m'améliorer et surtout d'écrire du code respectant la philosophie python  :jap:  

Message cité 1 fois
Reply

Marsh Posté le 09-10-2018 à 14:37:14   

Reply

Marsh Posté le 10-10-2018 à 21:30:52    

xilebo a écrit :

- mauvaise gestion des exceptions. Bien que ca existe en C++, je ne les ai jamais vraiment utilisées (on peut s'en passer ) sauf que c'est un peu obligatoire en python. Je ne sais pas quoi mettre dans try, except, else, et finally ( voir fonction r_parse_hex et r_parse_xml )


Rien du tout, sauf si tu veux spécifiquement gérer les erreurs correspondantes. Ici au lieu du try/finally tu voudras utiliser un "context manager"(similaire mais pas identique au RAII de C++) géré via with, et à la limite un gros try/except autour du tout si tu veux planquer les erreurs sans spécialement les gérer mais aussi sans arrêter complètement le processing.

 

Le "else" est également assez peu utilisé, ici on mettrait plus tout dans le body, on bien on utiliserait un early return:

Code :
  1. def r_parse_hex(e):
  2.     try:
  3.         with open(e['hex'], 'rb') as f:
  4.             ret = f.read(0x400) # lire entete eeprom
  5.             e['chk_hex'] = ret[0x212:0x214][:-1].hex().upper()    
  6.     except:
  7.         print("Error opening", e['hex'], file=sys.stderr)
  8.     return e
 

Prendre un objet qu'on modifie en place et renvoie c'est aussi un peu étrange, il y a des contextes où ça se fait (builders) mais ici ça semble pas trop être le cas.

xilebo a écrit :

- j'ai un peu bidouillé pour formatter ma string venant du fichier xml correctement en hex : j'ai paddé avec des 0 pour être sur d'avoir 4 caractères ( e['chk_xml'] = chk.attrib['hexa'].rjust(4,'0')   )


Bah si t'as besoin de 4 char et que ton entrée peut en avoir moins, faut padder. Tu pourrais le faire avec un format à la place mais tu gagnerais rien.

xilebo a écrit :


 - j'ai encore plus bidouiller pour formater la string venant du fichier hex :
   

Code :
  1. ret = f.read(0x400) # lire entete eeprom
  2.       e['chk_hex'] = ret[0x212:0x214][:-1].hex().upper()


             * je lis 1024 octets ( c'est normalement sur mais j'ai pas géré si inférieur )
             * mon checksum est à l'octet 0x212 + 0x213 mais à l'envers ( little endian ) , je l'inverse en rajoutant [:-1] puis transforme en string avec hex(), puis je passe en majuscule avec upper()


Alternativement tu peux parser ton truc avec struct.unpack (genre <H) et le formatter comme nécessaire derrière. Ou bien pas le formatter et parser la sortie de ton fichier XML avec int(val, 16).

xilebo a écrit :


 - mon parcours de fichier est comme ca :

Code :
  1. for root,dirs,files in os.walk("." ):       
  2.         for name in files:
  3.             fname = os.path.join(root, name)         
  4.             base , ext = os.path.splitext( name)           
  5.             if ext==".hex" or ext==".xml":               
  6.                 r_insert( result , ext[1:] , base , fname )
 

mais j'aurais préféré un truc du genre :

Code :
  1. [  os.path.join(dp, f) for dp, dn, fn in os.walk(os.path.expanduser("." )) for f in fn]


sauf que je ne sais pas comment rajouter dans la ligne du code permettant de filtrer d'une part les hex/xml, et appeler une fonction d'autre part à chaque itération pour effectuer le traitement.


S'pas trop possible à cause de la manière dont t'as structuré ton processing: ton r_insert va pas nécessairement générer de nouveaux éléments, et va potentiellement aller altérer des trucs en place à la place.

 

Si tu veux faire un truc du style, je suggère d'itérer juste sur les bases et pour chaque base aller chercher les données dans le fichier hex et le fichier xml en parallèle, ça évite des bricolages genre aller faire des recherches linéaires dans une liste. Apprends aussi à utiliser les fonctions de formattage Python (%, format ou f-strings).

 

Pas testé, mais un truc genre:

Code :
  1. def parse_xml(path):
  2.     try:
  3.         doc = ET.parse(path)
  4.         return (
  5.             doc.find('pnp_fin').attrib['label'],
  6.             int(doc.find('pnp_checksum'].attrib['hexa'], 16)
  7.         )
  8.     except Exception as e:
  9.         print(f"Failed to read {path}: {e}", file=sys.stderr)
  10.         return None, -1
  11.  
  12.  
  13. def parse_hex(path):
  14.     try:
  15.         with open(path, 'rb'):
  16.             [result] = struct.unpack_from("<H", path.read(0x400), 0x212)
  17.             return result
  18.     except Exception as e:
  19.         print(f"Failed to read {path}: {e}", file=sys.stderr)
  20.  
  21. if __name__ == '__main__':
  22.     for root, _, files in os.walk('.'):
  23.         for name in fnmatch.filter(files, '*.xml'):
  24.             fbase, _ = os.path.splitext(os.path.join(root, files))
  25.  
  26.             label, checksum_xml = parse_xml(fbase + '.xml')
  27.             checksum_hex = parse_hex(fbase + '.hex')
  28.  
  29.             if check_xml == check_hex:
  30.                 print(f"GOOD \t{label:>16}\t(chk={check_xml})" )
  31.             else:
  32.                 print(f"BAD \t{label:>16}\t(xml={check_xml},hex={check_hex})" )
 

Si tu veux absolument un itérateur ou une listcomp c'est moyen pratique à cause des manipulations de path avant de parser les fichiers. Ça serait faisable avec un helper ou bien la technique de la liste unaire (for foo in [val] c'est comme faire foo = 1 au milieu de la listcomp) mais je recommande pas spécialement.

 

D'ailleurs en regardant ce code, je me dis que la gestion d'erreur devrait aller autour des appels à parse_xml et parse_hex, si l'un des deux foire l'autre est pas utile:

Code :
  1. def parse_xml(path):
  2.     doc = ET.parse(path)
  3.     return (
  4.         doc.find('pnp_fin').attrib['label'],
  5.         int(doc.find('pnp_checksum'].attrib['hexa'], 16)
  6.     )
  7.  
  8. def parse_hex(path):
  9.     with open(path, 'rb'):
  10.         [result] = struct.unpack_from("<H", path.read(0x400), 0x212)
  11.         return result
  12.  
  13. if __name__ == '__main__':
  14.     for root, _, files in os.walk('.'):
  15.         for name in fnmatch.filter(files, '*.xml'):
  16.             fbase, _ = os.path.splitext(os.path.join(root, files))
  17.  
  18.             try:
  19.                 label, checksum_xml = parse_xml(fbase + '.xml')
  20.                 checksum_hex = parse_hex(fbase + '.hex')
  21.             except Exception as e:
  22.                 print(f"ERROR failed to load {fbase}: {e}" )
  23.             else:
  24.                 if check_xml == check_hex:
  25.                     print(f"GOOD \t{label:>16}\t(chk={check_xml})" )
  26.                 else:
  27.                     print(f"BAD \t{label:>16}\t(xml={check_xml},hex={check_hex})" )


Les fonctions de parsing s'occupent que de leur "happy path", et c'est le driver qui gère les erreurs de parsing.


Message édité par masklinn le 10-10-2018 à 21:35:00

---------------
Stick a parrot in a Call of Duty lobby, and you're gonna get a racist parrot. — Cody
Reply

Marsh Posté le 12-10-2018 à 14:04:40    

Merci beaucoup pour ces précisions, je comprends mieux et je vois que je ne suis pas complètement dans le faux.
 
J'ai appliqué quelques correctifs suite aux conseils. L'objet retourné, c'est juste quand j'ai prototypé mes fonctions , elles étaient vides, j'ai tendance à retourner un truc plutot que de mettre pass, mais effectivement ici c'est pas super utile.

Reply

Sujets relatifs:

Leave a Replay

Make sure you enter the(*)required information where indicate.HTML code is not allowed