SARC-187 - Provide a prompt for mila-drac manual matching. #61
Conversation
| @@ -234,6 +248,84 @@ def _matching_names(DLD_data, DD_persons, name_distance_delta_threshold): | |||
| # assert D_person_found[cc_source] == match # optional | |||
|
|
|||
|
|
|||
| # pylint: disable=too-many-nested-blocks | |||
| def _matching_names_with_prompt(DLD_data, DD_persons, name_distance_delta_threshold): | |||
There was a problem hiding this comment.
Je trouve ça étrange qu'on se retrouve avec 2 fonctions aussi similaire. J'aurais imaginé que _matching_names aurait été modifié pour supporter les matchs manuels lorsque nécessaire.
There was a problem hiding this comment.
Pour ceci, je trouve que les deux fonctions commencent à diverger.
La fonction initiale _matching_names utilise find_exact_bag_of_words_matches qui retourne des paires de potentiels matchs, puis itère sur ces paires pour les résoudre.
La fonction que j'ai ajoutée, _matching_names_with_prompt, utilise une nouvelle fonction find_best_word_matches qui, elle retourne les 10 meilleurs matchs pour chaque mila_display_name. Son interface est donc différente de find_exact_bag_of_words_matches, et j'en ai besoin car je dois utiliser ces 10 bests matchs ensuite dans le prompt (donc, je ne peux pas utiliser les paires de matchs, à la place).
Du coup, il me semble qu'on a là deux algorithmes assez divergents. N'est-ce pas préférable de les laisser dans des fonctions séparées ?
There was a problem hiding this comment.
Si je ne me trompe pas, on pourrait obtenir la même chose que _matching_names si on ignore les matches où le meilleur match à une distance d'édition trop grande. Dans ce cas, même si on travaille avec une liste de 10, si on regarde juste le premier ça revient au même que _matching_names. Est-ce que j'oublie certaines détails?
There was a problem hiding this comment.
En effet, on peut juste prendre le meilleur match parmi les 10 meilleurs, et ça permet de reproduire le comportement de l'ancienne fonction.
C'est fait dans l'avant-dernier commit, et le code a été rebasé puis corrigé dans le dernier commit.
| print(mila_display_name, "(matched with)", cc_match) | ||
| else: | ||
| print("(ignored)") | ||
| break |
There was a problem hiding this comment.
Il faudrait conserver les matchings manuels pour les sauvegarder dans un json.
There was a problem hiding this comment.
Comment choisir le chemin du fichier JSON ? Dans le ticket, tu suggères /secrets/mila_drac_matchings.json .
- Est-ce que j'utilise ce chemin tel quel (donc le dossier
secretssera créé si nécessaire dans le dossier d'exécution du script) ? - Ou bien, faut-il placer ce fichier dans un dossier spécifique défini dans une configuration ?
There was a problem hiding this comment.
Il semble qu'on devra envisager de sauvegarder les matchings manuels plutôt dans une base de données MongoDB. Mais je m'interroge encore sur la structure des données à stocker dans la base (quelle collection, quels dictionnaires ou listes dans la collection, stc).
There was a problem hiding this comment.
J'en ai discuté avec Pierre. Il a utilisé un fichier json finalement plutôt que la DB. Tu pourrais utiliser la fonction qu'il a écrit ici: https://github.com/mila-iqia/SARC/pull/63/files#diff-8c66ae22baf1b630c928b374f6c58cb1b1246eb8831b53fd6ae9f66ed6f6230aR314-R319. Le fichier json est donc déjà défini dans la config dans son PR.
Il va y avoir un gros conflit entre vos PR. Pierre a beaucoup amélioré le code déjà existant en même temps qu'il a ajouté le code pour le matching de superviseurs. Je crois que ça serait bien que tu rebase ta branche sur la sienne et que tu utilise le fichier json tel que défini dans son PR.
notoraptor
force-pushed
the
sarc-187-jumelage-drac
branch
from
bdcab12
to
85ae993
Compare
notoraptor
force-pushed
the
sarc-187-jumelage-drac
branch
from
85ae993
to
a1b8c31
Compare
|
Salut @nurbal ! Vu que la PR de Pierre a été mergée, j'ai rebasé celle-ci. Ça devrait faciliter le reviewing. Il restait un commentaire de Xavier ( #61 (comment) ) suggérant de sauvegarder les matchings manuels dans un fichier JSON. Mais je ne sais pas exactement quel fichier utiliser. Dois-je en prendre un parmi ceux déjà dans la config, ou bien dois-je en ajouter un nouveau ? |
Il faut utiliser le contenu du répertoire Line 108 in 8e06b74 |
…to use it even without prompt.
notoraptor
force-pushed
the
sarc-187-jumelage-drac
branch
from
a1b8c31
to
0e2d731
Compare
|
@nurbal PR mise à jour ! J'ai aussi essayé d'étendre les tests pour vérifier que les nouveaux matchs sont bien sauvegardés dans le fichier JSON. J'ai essayé d'écrire un mock, je ne sais pas si c'est fait de la meilleure façon. |
notoraptor
force-pushed
the
sarc-187-jumelage-drac
branch
from
0940717
to
c51b67b
Compare
|
Unit tests corrigés, tous les tests passent ! |
Hi @bouthilx ! This is a first code for SARC-187. Already working, but not sure about the interface. What do you think ?