PnX-SI / GeoNature-citizen

Portail web d'inventaire citoyen de la biodiversité à destination du grand public
https://pnx-si.github.io/GeoNature-citizen/
GNU Affero General Public License v3.0
20 stars 29 forks source link

[Observations] Amélioration de la fonctionnalité autocomplete de taxon #327

Open mvergez opened 2 years ago

mvergez commented 2 years ago

Contexte

Suite à #323, lorsqu'une liste taxonomique comporte plus de 100 taxons, la recherche est peu efficace voire ne fonctionne pas. Ceci implique de reprendre la fonctionnalité d'autocomplete. Dans l'état actuel, l'autocomplete suit les étapes suivantes :

Proposition

Suite à #324, @TheoLechemia a proposé d'utiliser au maximum cette route (api/taxref/allnamebylist/) : https://github.com/PnX-SI/TaxHub/blob/master/apptax/taxonomie/routestaxref.py#L341 qui supporte la recherche de taxon (par le nom et par le cd_nom par exemple). Nous souhaitons donc l'utiliser pour remplacer l'autocomplete.

Implémentation

Nous pensons principalement agir sur ces lignes dans le cadre d'une potentielle prestation avec @jonath35

https://github.com/PnX-SI/GeoNature-citizen/blob/2f937ff8e415216af0050fc135019ea600fa9c37/frontend/src/app/programs/observations/form/form.component.ts#L164-L178 En appelant directement la route api/taxref/allnamebylist/ de taxhub avec un paramètre ?search_name=<taxon_ou_cd_nom>

Problèmes

Cette route fonctionne par code_liste et non par id_liste, il faudrait donc, sous les conseils de @TheoLechemia, fonctionner par code_liste, cela implique donc un changement dans la base de données de citizen. En effet, actuellement seul l'id_liste est stocké par programme.

Conséquences

Ceci impliquera une absence de média dans les propositions de l'autocomplete qui n'aide, selon @Adrien-Pajot, pas l'utilisateur à l'identification de l'espèce. Nous proposons dans un nouveau ticket (#328) d'ajouter le média après la sélection par select ou autocomplete sur la droite de ces derniers

TheoLechemia commented 2 years ago

l'id liste à été copié dans le code liste dans une des dernières MAJ de Taxhub. Il suffirait de faire ça non ?

mvergez commented 2 years ago

Salut Théo,

l'id liste à été copié dans le code liste dans une des dernières MAJ de Taxhub. Il suffirait de faire ça non ?

Je ne suis pas sûr de comprendre cette copie de l'id_liste vers code_liste ? Que veux-tu entendre par là ?

TheoLechemia commented 2 years ago

la valeur de l'id liste a été copié dans la colonne code_liste pour assurer une retrocompatibilité.

mvergez commented 2 years ago

Ok, je comprends. Le problème, c'est que dans citizen, on stocke un id_liste (int) et pas un code_liste(varchar). Cela amène donc cette modification mineure dans la base de données de citizen et une clarification dans la doc. Mais ce qui est top, c'est que la rétrocompatibilité sera assurée grâce à la copie dont tu parles

lpofredc commented 2 years ago

@TheoLechemia, est-il prévu d'étendre l'utilisation de ce code_list à toutes les routes faisant appel à une liste, ou faut-îl quand même récupérer l'id_liste pour d'autres usages. Par exemple les routes /api/biblistes/<id_liste>....

Si l'id_liste reste privilégié pour ces routes, ne faudrait-il pas plutôt continuer à stocker cet id_liste mais demander au backend de récupére ce code_liste pour l'envoyer au front avec les autres infos générales du programme.

TheoLechemia commented 2 years ago

Je dirait qu'il faudrait utiliser le code_liste partout. Mais comme l'id_liste a été copié dans le code_liste c'est rétrocompatible partout. Dans citizen vous pouvez l'appelez avec votre id_liste et ça fonctionnera

lpofredc commented 2 years ago

Pour les listes historiques, cela fonctionne, mais maintenant que le code_liste est personnalisable, on va se retrouver avec des nouvelles listes ou l'id_liste est différent du code_list.

image

camillemonchicourt commented 2 years ago

Oui c'est embêtant en effet.

maxreinhart commented 2 years ago

Bonjour tout le monde, je ne sais pas à quel point il y a une volonté forte de ne plus utiliser le id_list au profit du code_list, mais on pourrait implémenter une nouvelle vue get_AllTaxrefNameByIDListe (en plus de celui déjà existant, avec du code partagé entre les deux) pour se baser sur le id_list à la place de code_list ?

Surtout que la première chose que fait get_AllTaxrefNameByListe, c'est de récupérer un id_list à partir du code_list pour effectuer son filtrage.

Sinon une autre solution serait de proposer une vue dans TaxHub pour retourner le code_list d'une liste d'espèces à partir d'un id_list.

camillemonchicourt commented 2 years ago

On utilise en général plutôt les codes que les listes quand on a besoin d'identifiants communs qui sont partagés et communs entre différentes bases de données. Dans le cas des identifiants des listes de taxons pour un programme de GeoNature-citizen il y a peu d'enjeux et d'intérêt à avoir des identifiants communs et partagés donc peu importe selon moi.

Selon moi il ne devrait pas y avoir une autre route pour filtrer par code_liste. Je pense même qu'il ne devrait pas y avoir de route /allnamebylist/ comme actuellement. Selon moi il devrait juste y avoir une route /names/ plus générique filtrable par id_liste, code_liste, rang taxonomique, cd_ref, etc...

andriacap commented 10 months ago

Bonjour,

Je reviens sur ce sujet qui commence à dater. Est ce que les idées, positions , remarques ont évoluées ? Est ce que l'utilisation de la route api/taxref/allnamebylist/ à la place du mécanisme de recherche de taxons actuel (comme mentionné dans la description de l'issue) est envisageable en terme de développement ? Merci d'avance

camillemonchicourt commented 10 months ago

Tout cela a été repris récemment au niveau de TaxHub en revenant aux id_listes et en faisant évoluer la route /taxref pour ne plus utiliser que cette dernière dans Occtax-mobile. Il faut que je retrouve un peu les tickets où on en parle pour remettre ça au clair dans ma tête.

camillemonchicourt commented 10 months ago

OK trouvé, c'est expliqué ici - https://github.com/PnX-SI/TaxHub/issues/346

andriacap commented 10 months ago

Merci Camille pour ton retour. Du coup, la modification de la recherche de txon dans citizen en utilisant la route allnamebylist/<id_liste> reste pertinent ?

camillemonchicourt commented 10 months ago

Oui c'est ça.

camillemonchicourt commented 10 months ago

Mais à analyser, car il vaut mieux privilégier l'utilisation de la route principale /taxref, plutôt que /allnamebylist qui est plus spécifique. Mais l'avantage de /allnamebylist est qu'elle est bien taillée pour de l'autocomplétion et renvoie à plat Nom Vern - Nom scientifique [cd_nom]. Donc c'est peut-être celle-ci dont vous avez besoin.

andriacap commented 7 months ago

Bonjour,

Après réflexion et discussion avec @lpofredc il est ressorti que le plus simple serait d'ajouter la gestion de params pour l route d'autcompletion liée à TaxHub `allnamebylist`` https://github.com/PnX-SI/TaxHub/blob/ca38000d89c074cfba9c80965ab11d9ef679c3ce/apptax/taxonomie/routestaxref.py#L378-L396

L'idée étant de rajouter la possibilité de récupérer les médias (photo_principale) et le "nom_français" . Pourquoi vouloir ajouter ces pararms ? Car dans GN Citizen on doit récupérer les médias et les nom français associés aux taxons pour les afficher coté frontend .

Cette modification permettrait d'éviter de complexifier les appels en chaine de différentes routes pour récupérer d'une part les taxons (cd_nom etc) et d'autre part les médias associés

Si c'est ok , on partirait sur une modif coté TaxHub (sur la route/allnamebylist) et une modif coté GN Citizen Merci pour vos retours.

@camillemonchicourt @lpofredc

camillemonchicourt commented 7 months ago

Je ne comprends pas le rapport avec les médias, etc. Là l'objectif est d'avoir une liste auto-complétion pour des programmes avec beaucoup beaucoup de taxons (quasi tout Taxref) et en mode auto-complétion on n'affiche pas les médias des taxons et on renvoie nom FR et nom scientifique comme le fait déjà la route "allnamebylist".

hypsug0 commented 7 months ago

Il y a un peu plus d'implications que ça.

Tout d'abord, l'objectif final serait de remplacer purement et simplement toutes les routines du backend qui génèrent actuellement les listes d'espèces par le cette route de taxhub qui serait donc ainsi directement consultée par le frontend plutôt que de conserver un système hybride délégué au backend pour les listes courtes et au frontend pour les listes longues. De fait, on a a minima besoin des médias pour les vignettes pour les petites listes d'espèces (filtrés par type de média).

Ensuite, on a également besoin des médias pour les vignettes des espèces dans utilisées dans la liste des obs et dans les popup de la carte. Là, je ne sais pas encore comment gérer convenablement ce cas de figure d'énormes listes. C'est de mon point de vue un non sens des listes de plusieurs milliers de taxons dans citizen...

Et on a toujours besoin du nom français qui disparaitra de bib_noms > vers un attribut ?

camillemonchicourt commented 7 months ago

Oui le fait de ne plus dupliquer des routes de taxonomie dans GN-citizen et directement interroger les routes existantes de TaxHub est un sujet identifié et déjà initié, qu'il est intéressant de poursuivre.

Par contre, si on veut des infos brutes et plus complètes, il faut peut-être plutôt utilisée la route /taxref qui a été enrichie depuis la version 1.13.1 de TaxHub (https://github.com/PnX-SI/TaxHub/pull/451).

La route /allnamebylist est vraiment dédiée à la recherche par auto-complétion combinant nom vernaculaire, nom scientifique et cd_nom.

Selon moi, /allnamebylist devrait être la plus légère possible et je ne vois même pas pourquoi elle renvoie "nom_valide", "lb_nom", "nom_vern", "regne", "group2_inpn", "group3_inpn". Peut-être pour des filtres mais dans ce cas ça devrait plutôt être des paramètres de filtres de la route. Mais je n'ai pas tout en tête.

hypsug0 commented 7 months ago

Salut, merci, en effet, cette route /taxref avec filtrage des champs retournés est vraiment pas mal pour générer nos listes d'espèces.

Il manque toutefois toujours la capacité à récupérer facilement les médias et attributs associés aux taxons, par id_liste.

camillemonchicourt commented 7 months ago

Oui, à discuter côté TaxHub, mais on peut peut-être envisager d'ajouter à la route /taxref la possibilité de renvoyer les médias et peut-être les attributs. Peut-être pas par défaut, mais seulement si les demande ces champs, pour ne pas l'alourdir par défaut ? 🤔 Et car on s'éloigne de ce qui est fourni par Taxref.

hypsug0 commented 7 months ago

Oui! :+1: , c'est bien l'idée. En effet, à discuter sur taxhub.

andriacap commented 2 weeks ago

Bonjour,

Je fais suite aux échanges sur le sujet qu'll y a eu sur Element avec notamment @camillemonchicourt et @hypsug0 .

La dernière version de taxhub sortie il y a deux semaines permet de récupérer les medias depuis la route `/taxref'. Citizen doit pouvoir être compatible avec TaxHub V2 et donc tout ce qui touche à bibnoms sera supprimé dans citizen. La fonctionnalité de badge n'étant pas fonctionnelle en l'état n'est pas concerné par les développements proposés.

Voici les propositions d'évolutions de citizen pour pouvoir avoir des listes de plus de 100 taxons ( des centaines de milliers de taxons).

L'ensemble des taxons des listes de chaque programme ne sont plus mis en cache dans citizen. L'option priviélgié ici est le lazy loading pour appeler les listes uniquement aux endroits où il est nécessaire d'avoir les informations.

L'appel à la route biblistes dans citizen doit pouvoir récupérer le nombre de taxons de chaque liste pour s'en servir sur l'affichage conditionnel du composant de recherche d'espèces et sur l'affichage des observations d'un programme( voir code à modifier : https://github.com/PnX-SI/GeoNature-citizen/blob/68469ff72e48bd0ae47ff668ad701219026efeac/backend/gncitizen/utils/taxonomy.py#L59) ajout de count présent (voir coté taxhub : https://github.com/PnX-SI/TaxHub/blob/ea9434de5a1f227131e0e8640ad17f8a25e8a39d/apptax/taxonomie/routesbiblistes.py#L27)