diff --git a/scripts/deduplication/deduplicate.py b/scripts/deduplication/deduplicate.py index 9c15b5d13..acb27a1f4 100755 --- a/scripts/deduplication/deduplicate.py +++ b/scripts/deduplication/deduplicate.py @@ -23,8 +23,9 @@ from tasks.db_manage_cluster import db_manage_cluster from tasks.source_data_get import source_data_get from utils.cli import banner +from utils.db import db_source_ids_by_code_get -from qfdmo.models import RevisionActeur, Source +from qfdmo.models import RevisionActeur install() # you can install globally to your env, see docs @@ -36,7 +37,10 @@ # A adapter RUN_SOURCES_PREFERRED_CODES = ["ALIAPUR", "COREPILE"] RUN_ID = "dechetteries_202412" -RUN_CLUSTER_IDS_TO_SKIP = [] # Si besoin pour passer des erreurs +RUN_CLUSTER_IDS_TO_SKIP = [ + "85250_3_1", + "93320_1_1", +] # Si besoin pour passer des erreurs # Automatique par le script RUN_CLUSTER_IDS_TO_CHANGES = getattr( @@ -146,7 +150,7 @@ def main() -> None: # ------------------------------------------ # Pour privilégier certaines sources au moment de la création des parents banner("RECUPERATION IDS DES SOURCES PREFEREES") - source_ids_by_codes = dict(Source.objects.values_list("code", "id")) + source_ids_by_codes = db_source_ids_by_code_get() sources_preferred_ids = [ source_ids_by_codes[x] for x in RUN_SOURCES_PREFERRED_CODES ] diff --git a/scripts/deduplication/tasks/db_manage_parent.py b/scripts/deduplication/tasks/db_manage_parent.py index 0df399363..a896d6b98 100644 --- a/scripts/deduplication/tasks/db_manage_parent.py +++ b/scripts/deduplication/tasks/db_manage_parent.py @@ -14,6 +14,7 @@ from qfdmo.models.acteur import RevisionActeur from scripts.deduplication.models.acteur_map import ActeurMap from scripts.deduplication.models.change import Change +from scripts.deduplication.utils.db import db_source_ids_by_code_get def parent_id_generate(ids: list[str]) -> str: @@ -102,8 +103,11 @@ def acteurs_dict_to_list_of_dicts(acteurs_maps: list[ActeurMap]) -> list[dict]: def parent_get_data_from_acteurs( - acteurs: list[dict], sources_preferred_ids, validation_model: Model -) -> dict: + acteurs: list[dict], + source_ids_by_code: dict, + sources_preferred_ids, + validation_model: Model, +) -> tuple[dict, dict]: """ Récupération des données d'acteurs pour l'utilisation dans un parent. Pour l'instant la logique est primitive de choisir selon une liste de sources préférées. A l'avenir @@ -116,8 +120,88 @@ def parent_get_data_from_acteurs( Returns: parent_dict: donnée parent sous forme de dict + sources_codes_picked: mapping des champs et sources choisies """ + # TODO: revoir les règles d'inclusion/exclusion pour la prochaine + # dédup pour le rendre plus flexible avec l'acteur_type_id + # - entrée de mapping = acteur_type_id x champ + # - sortie de mapping = liste de sources à inclure/exclure + if any(x["acteur_type_id"] not in (None, 7) for x in acteurs): + raise NotImplementedError( + """Règles inclusion/exclusion pour la dédup déchetteries uniquement, + revoir avec Christian avec la prochaine dédup""" + ) + + source_codes_by_id = {v: k for k, v in source_ids_by_code.items()} parent_dict = {} + sources_codes_picked = {} + # Mapping d'inclusion de source spécifique à certains champs + # TODO: les 2 mapping doivent être exclusif + field_source_codes_priority = { + "nom": ["COREPILE", "ALIAPUR"], + "location": ["REFASHION"], + } + # Mapping d'exclusion de source spécifique à certains champs + field_source_codes_exclusion = { + "nom": ["REFASHION"], + "nom_commercial": ["REFASHION", "ECODDS"], + "siret": ["REFASHION"], + "siren": ["REFASHION"], + "telephone": ["REFASHION"], + } + # Ignore les champs d'identifications acteurs + # car il doivent tous restés vides (générés ultérieurement) + keys_to_ignore = [ + "identifiant_unique", + "identifiant_externe", + "parent_id", + "source_id", + # Déjà géré par Django + "modifie_le", + # Soit on garde la valeur historique, ou soit pour un nouveau + # parent c'est géré par Django + "cree_le", + ] + + def results_update(field, value, acteur): + """utilitaire MAJ de parent_dict et sources_codes_picked + + some debug prints pour éviter de se répéter sur les + 2 boucles""" + source_id = acteur["source_id"] + source_code = source_codes_by_id.get(source_id) + acteur_id = acteur["identifiant_unique"] + parent_dict[field] = value + sources_codes_picked[field] = source_code + print(f"\t✅ {field=} {value=}") + print(f"\t\t via {source_code=} {acteur_id=}") + + # ------------------------------------ + # Boucle 1: données via inclusion/exclusion champs/sources spécéfiques + # ------------------------------------ + print("parent_get_data_from_acteurs: boucle 1") + for field, source_codes_inclusion in field_source_codes_priority.items(): + source_codes_exlusion = field_source_codes_exclusion.get(field, []) + # acteurs candidats pour le champ + cands = sorted( + [ + x + for x in acteurs + if x["source_id"] is not None + and source_codes_by_id[x["source_id"]] in source_codes_inclusion + and source_codes_by_id[x["source_id"]] not in source_codes_exlusion + and x[field] is not None + ], + key=lambda x: source_codes_inclusion.index( + source_codes_by_id[x["source_id"]] + ), + ) + acteur = next(iter(cands), None) + if acteur is not None: + results_update(field, acteur[field], acteur) + + # ------------------------------------ + # Boucle 2: données restantes via les sources préférées + # ------------------------------------ # Ordonner acteurs sur source_id dans l'ordre de sources_preferred_ids # et les autres acteurs par défaut après acteurs = sorted( @@ -132,47 +216,44 @@ def parent_get_data_from_acteurs( else len(sources_preferred_ids) ), ) - # Ignore les champs d'identifications acteurs - # car il doivent tous restés vides (sauf identifiant_unique - # qui est généré avec un UUID séparément) - keys_to_ignore = [ - "identifiant_unique", - "identifiant_externe", - "parent_id", - "source_id", - ] - print("parent_get_data_from_acteurs:") + print("parent_get_data_from_acteurs: boucle 2") email_field = forms.EmailField() for acteur in acteurs: - for key, val in acteur.items(): - if key in keys_to_ignore: - continue - if val is not None and parent_dict.get(key) is None: + source_code = source_codes_by_id.get(acteur["source_id"]) + # Précédent parent non selectionné comme parent pour le nouveau + # cluster, on veut pas prendre le risque de réutiliser ces données + if source_code is None: + continue + for field, value in acteur.items(): + source_codes_exlusion = field_source_codes_exclusion.get(field, []) + if ( + field not in keys_to_ignore + and value is not None + and parent_dict.get(field) is None + and source_code not in source_codes_exlusion + ): # TODO: voir si il y aurait une façon plus élégante # d'automatiquement supprimer toutes les données invalides # plutôt que de le faire manuellement champ par champ. # Ceci est en lien avec le problème de présence de mauvais # emails dans la DB, qui empêche la réutilisation du # modèle django car .save() appelle .full_clean() qui crash - if key == "email": + if field == "email": try: - email_field.clean(val) + email_field.clean(value) except forms.ValidationError: continue - print( - f"\t{key=}, {val=}", - f"via {acteur['source_id']=} {acteur['identifiant_unique']=}", - ) - parent_dict[key] = val + results_update(field, value, acteur) # Et on met la source_id à None car c'est une création de notre # part, donc il ne correspond pas à une source existante parent_dict["source_id"] = None + # On s'assure que la donnée est compatible avec le modèle de révision validation_model(**parent_dict) # type: ignore # On retourne la données sous forme de dict pour qu'elle puisse # être utilisée soit sur un parent existant (update) soit pour créer un # nouveau parent (insert) - return parent_dict + return parent_dict, sources_codes_picked def db_manage_parent( @@ -202,8 +283,11 @@ def db_manage_parent( identifiants_uniques = [x.identifiant_unique for x in acteurs_maps] acteurs_dicts = acteurs_dict_to_list_of_dicts(acteurs_maps) # print(f"{acteurs_list=}") - parent_data = parent_get_data_from_acteurs( - acteurs_dicts, sources_preferred_ids, validation_model=RevisionActeur # type: ignore + parent_data, _ = parent_get_data_from_acteurs( + acteurs=acteurs_dicts, + source_ids_by_code=db_source_ids_by_code_get(), + sources_preferred_ids=sources_preferred_ids, + validation_model=RevisionActeur, # type: ignore ) print(f"{parent_data=}") diff --git a/scripts/deduplication/utils/db.py b/scripts/deduplication/utils/db.py new file mode 100755 index 000000000..c3153abad --- /dev/null +++ b/scripts/deduplication/utils/db.py @@ -0,0 +1,26 @@ +"""Utilitaires pour les interactions DB""" + +from functools import cache + +from qfdmo.models.acteur import Source + + +# A propos de @cache: +# Données de très petite taille donc unbounded cache est OK +# Raison pour le cache: +# - pouvoir utiliser le mapping dans des sous-tâches sans +# avoir à se trimballer des arguments partout +# (manage cluster -> manage parent) +# - gain de performance: au 2024-12-18 il nous faut 2h23min pour +# traiter n=4700 clusters, preuve que l'accès DB est assez lent +# et donc économiser n=requêtes (mapping utilisé à la fois dans +# db_manage_cluster et db_manage_parent) ne parait pas scandaleux +@cache +def db_source_ids_by_code_get() -> dict: + """Récupération de la liste des sources par code pour les utiliser + dans la logique de priorisation des sources pour la fusion des données + + Returns: + result: dict de sources code:id + """ + return dict(Source.objects.values_list("code", "id")) diff --git a/scripts/tests/deduplication/tasks/test_parent_get_data_from_acteurs.py b/scripts/tests/deduplication/tasks/test_parent_get_data_from_acteurs.py index b0938cb0d..d5a52e939 100644 --- a/scripts/tests/deduplication/tasks/test_parent_get_data_from_acteurs.py +++ b/scripts/tests/deduplication/tasks/test_parent_get_data_from_acteurs.py @@ -1,62 +1,209 @@ -""" +"""" Test de la fonction parent_get_data_from_acteurs qui permet de fusionner les données de plusieurs acteurs pour en faire un parent, en priorisant les sources de préférence puis les données les plus complètes. + + +Cas de figures à tester: + - un champ n'est jamais rempli si toutes les valeurs dispos sont None + - le source_id est mis à None pour expliciter la non-source (càd parent fabriqué) + - les autres id (identifiant, parent_id) ne doivent pas être remplis + - location: on priorise une source autre que les sources par défaut + - email: on ne prend pas de mauvais email """ +import pytest +from django.contrib.gis.geos import Point + from qfdmo.models import RevisionActeur from scripts.deduplication.tasks.db_manage_parent import parent_get_data_from_acteurs +# Pour l'instant notre code a été conçu pour les déchetteries +ACTEUR_TYPE_ID = 7 + +SOURCE_IDS_BY_CODES = { + # A noter le code REFASHION tel que + # présent dans la base (son id peut différé) + # parce que la règle de résolution des champs + # est pour l'instant en dure dans + "REFASHION": 123456789, + "ALIAPUR": 456, + "COREPILE": 46511111, + "Bon url": 654, + "Bon email & siret": 789, + "Bon nom": 111, +} +SOURCES_PREFERRED_IDS = [ + SOURCE_IDS_BY_CODES["Bon nom"], + SOURCE_IDS_BY_CODES["Bon url"], +] +SOURCE_CODES_PICKED_EXPECTED = { + "nom": "COREPILE", + "url": "Bon url", + "location": "REFASHION", + "siret": "Bon email & siret", + "email": "Bon email & siret", + # C'est la première source qui a été choisie + # pour l'acteur type + "acteur_type_id": "Bon url", +} +PARENT_EXPECTED = { + # Intentionnellement pas de identifiant_unique retourné + # pour nous forcer à gérer le ID séparément (ex: génréer un UUID) + "nom": "nom de COREPILE", + "url": "url_a", + "siret": 11111111111111, + "location": Point(1, 2), + # Le source_id est nulle parce que le parent est généré par nous + # donc ne provient d'aucune source en particulier + "source_id": None, + "acteur_type_id": ACTEUR_TYPE_ID, +} + + +class TestParentDataFromActeur: + + @pytest.fixture(scope="session") + def acteurs_get(self) -> list[dict]: + return [ + { + "identifiant_externe": "A", + "identifiant_unique": "a", + "adresse_complement": None, + "nom": "Mon A", + "url": "url_a", + "siren": None, + "source_id": SOURCE_IDS_BY_CODES["Bon url"], + "parent_id": None, + "acteur_type_id": ACTEUR_TYPE_ID, + "cree_le": "2021-01-01", + "modifie_le": "2021-01-01", + }, + { + "identifiant_externe": "B", + "identifiant_unique": "b", + "adresse_complement": None, + "nom": "Mon B", + "url": None, + "siret": 12345678912345, + "siren": 123456789, + "telephone": "02 43 56 78 90", + "source_id": SOURCE_IDS_BY_CODES["REFASHION"], + "parent_id": "p1", + "location": Point(1, 2), + "acteur_type_id": ACTEUR_TYPE_ID, + }, + { + "identifiant_externe": "C", + "identifiant_unique": "c", + "nom": "nom de COREPILE", + "url": None, + "siret": None, + "source_id": SOURCE_IDS_BY_CODES["COREPILE"], + "parent_id": "p2", + "email": "INVALID", + # location non-nulle sur la source id préférée (3) + # mais qui sera ignorée car source 45 (REFASHION) + # est prioritaire sur les locations + "location": Point(0, 0), + "acteur_type_id": ACTEUR_TYPE_ID, + }, + { + "identifiant_externe": "D", + "identifiant_unique": "d", + "nom": "Mon D", + "url": None, + "siret": 11111111111111, + "source_id": SOURCE_IDS_BY_CODES["Bon email & siret"], + "parent_id": "p2", + # on met le bon email sur une location + "email": "correct@example.org", + "acteur_type_id": ACTEUR_TYPE_ID, + }, + ] + + @pytest.fixture(scope="session") + def results_get(self, acteurs_get) -> tuple[dict, dict]: + + return parent_get_data_from_acteurs( + acteurs=acteurs_get, + source_ids_by_code=SOURCE_IDS_BY_CODES, + sources_preferred_ids=SOURCES_PREFERRED_IDS, + validation_model=RevisionActeur, # type: ignore + ) + + @pytest.fixture(scope="session") + def parent(self, results_get) -> dict: + parent, _ = results_get + return parent + + @pytest.fixture(scope="session") + def source_codes_picked(self, results_get) -> dict: + _, source_codes_picked = results_get + return source_codes_picked + + def test_field_never_filled_because_all_none(self, parent): + """On ne remplit pas un champs si toutes les valeurs dispos sont None""" + assert "adresse_complement" not in parent + + def test_datetimes_are_removed(self, parent): + """Les dates de création et modification ne sont pas copiées""" + assert "cree_le" not in parent + assert "modifie_le" not in parent + + def test_source_id_set_to_none(self, parent): + """Pour expliciter que le parent a été fabriqué par nous""" + assert parent["source_id"] is None + + def test_acteur_type_id_remains_same(self, parent): + """L'acteur type est conservé""" + assert parent["acteur_type_id"] == ACTEUR_TYPE_ID + + def test_ids_not_present_except_source_id(self, parent): + """On ne renseigne pas les autres ids, qui seront + assignés plus tard (id parent existant ou UUID pour nouveau)""" + assert "parent_id" not in parent + assert "identifiant_unique" not in parent + + def test_email_picked_is_valid(self, parent): + """On a réussi à exclure le mauvais email""" + assert parent["email"] == "correct@example.org" + + def test_field_mapping_source_picked_preferred(self, source_codes_picked): + """Pour le location, on a réussi à avoir la source qui était + n1 pour ce champ""" + assert source_codes_picked["location"] == "REFASHION" + + def test_field_mapping_source_picked_fallback(self, source_codes_picked): + """Pour le nom, on avait la source préférée, mais a réussi notre fallback + sur la 2ème""" + assert source_codes_picked["nom"] == "COREPILE" + + def test_field_mapping_source_exclusions(self, source_codes_picked): + """Pour plusieurs champs, on a réussi à exclure la source qui avait un siret + non-nul mais invalide""" + assert source_codes_picked["siret"] != "REFASHION" + assert source_codes_picked["siret"] == "Bon email & siret" + + assert "siren" not in source_codes_picked + assert "telephone" not in source_codes_picked + + def test_source_ids_picked(self, results_get): + """Vérification d'ensemble sur les sources choisies pour + s'assurer qu'il n'y a pas des champs définis dans + SOURCE_CODES_PICKED_EXPECTED qui sont passés aux oubliettes""" + _, source_codes_picked = results_get + assert source_codes_picked == SOURCE_CODES_PICKED_EXPECTED -def test_parent_get_data_from_acteurs(): - acteurs = [ - { - "identifiant_externe": "A", - "identifiant_unique": "a", - "nom": "Mon A", - "url": "url_a", - "siren": None, - "source_id": 1, - "parent_id": None, - }, - { - "identifiant_externe": "B", - "identifiant_unique": "b", - "nom": "Mon B", - "url": None, - "siret": 12345678912345, - "source_id": 2, - "parent_id": "p1", - }, - { - "identifiant_externe": "C", - "identifiant_unique": "c", - "nom": "Mon C", - "url": None, - "siret": None, - "source_id": 3, - "parent_id": "p2", - "email": "INVALID", - }, - { - "identifiant_externe": "D", - "identifiant_unique": "d", - "nom": "Mon D", - "url": None, - "siret": None, - "source_id": 4, - "parent_id": "p2", - "email": "INVALID", - }, - ] - source_ids_preferred = [3, 2] - parent_expected = { - # Intentionnellement pas de identifiant_unique retourné - # pour nous forcer à gérer le ID séparément (ex: génréer un UUID) - "nom": "Mon C", - "url": "url_a", - "siret": 12345678912345, - "source_id": None, - } - parent = parent_get_data_from_acteurs(acteurs, source_ids_preferred, RevisionActeur) # type: ignore - assert parent == parent_expected + def test_parent_dict(self, parent): + # On ne peut pas comparer les dicts + # en entier directement à cause de "location" + # dont le hash est une ref mémoire, et donc non-deterministe + # Point(1, 2) <=> Point(1, 2) + # {'location': } <=> + # {'location': } + for key in PARENT_EXPECTED: + if key == "location": + assert parent[key].coords == PARENT_EXPECTED[key].coords + else: + assert parent[key] == PARENT_EXPECTED[key]