flotpython / course

Contenu pédagogique du MOOC Python3 sur fun-mooc.fr
Other
136 stars 22 forks source link

classe Roman : des corrections et des commentaires #41

Closed aollier closed 3 years ago

aollier commented 3 years ago

J'ai fait plusieurs corrections séparées dans des commits différents, et j'ai ajouté des commentaires pour rendre le tout un peu plus compréhensible de prime abord. J'espère que cela te convient @parmentelat.

aollier commented 3 years ago

J'attends qu'on trouve des tests supplémentaires pour les pousser dans cette pull request et la rendre fusionnable.

parmentelat commented 3 years ago

super, merci Adrien

s'agissant des tests, ça me va déjà très bien qu'on ait enlevé les exemples grossièrement faux :)

après, je ne pense pas que ça vaille spécialement le coup d'insister trop sur les tests correspondant aux opérations arithmétiques, mais on pourrait vérifier que la conversion se passe bien, et dans les deux sens

en fait j'avais déjà bricolé un truc dans la dernière cellule du notebook qui allait un peu dans ce sens je ne suis pas sûr de bien couvrir tous les cas limite, mais une liste par exemple :

MDCCCIX=1809 MDCCCXIX=1819 MDCCCXXIX=1829 MDCCCXXXIX=1839 MDCCCXLIX=1849 MDCCCLIX=1859 MDCCCLXIX=1869 MDCCCLXXIX=1879 MDCCCLXXXIX=1889 MDCCCXCIX=1899 MCMXXXIX=1939 MCMXL=1940 MCMXLI=1941 MCMXLIX=1949 MCML=1950 MCMLI=1951 MCMLIX=1959 MCMLX=1960 MCMLXI=1961 MCMXCVIII=1998 MCMXCIX=1999 MM=2000 MMI=2001 MMIX=2009 MMX=2010 MMXI=2011 MMCMXLIX=2949 MMCMIX=2909 MMCMXCIX=2999 MMMCMXCIX=3999

aollier commented 3 years ago

Oui, ton idée est très bien. Pour simplifier le test et tester dans les deux sens, est-ce qu'il est possible d'écrire quelque chose comme ceci ?

from corrections.cls_roman import Roman as Rmn

for i in range(2801):
    r1 = Roman(i)
    r2 = Rmn(i)
    if repr(r1) != repr(r2):
        print(f"OOPS with {repr(r1)} != {repr(r2)}")
    r1 = Roman(str(r2))
    if repr(r1) != repr(r2):
        print(f"OOPS with {repr(r1)} != {repr(r2)}")

On peut limiter l'étendue si tu trouves que 2801 tests c'est trop (j'inclus volontairement le nombre zéro pour lequel repr(Rmn(0)) == "N=nan").

parmentelat commented 3 years ago

disons: dans l'état actuel de l'exo, il y a

  1. la batterie de tests automatiques; je trouve qu'il n'y a pas suffisamment
  2. et du coup j'avais rajouté, mais dans le notebook cette fois, une cellule de code pour debugger

ce que j'aimerais bien c'est qu'on étende un peu mieux le 1. mais sans aller jusqu'à en mettre des centaines non plus

après, on peut tout à fait garder l'idée de la cellule de debug, et cette fois faire un truc très extensif, oui pas de souci par contre je préfèrerais ne pas importer la classe des solutions dans toute la mesure du possible... on n'en a pas complètement besoin si on câble en dur les associations decimal-roman..

parmentelat commented 3 years ago

je me suis occupé de merger et d'ajouter les tests

merci bien Adrien !

parmentelat commented 3 years ago

j'ai aussi mis à jour à l'instant les corrigés, en principe ils sont tous à jour avec le répo d'aujourd'hui

aollier commented 3 years ago

Avec plaisir ! 😄