Linked issue : #2001
feat: add ban addresses to photon results #2365
6be438922f
to 349d4ce117
@MaGOs92 n'hésite pas à procéder par commits successifs, ça m'aidera pour les prochaines revues :)
Oups pardon, il y a bien un deuxième commit, je ne l'ai pas vu avant d'aller dans l'onglet revisions, autant pour moi.
@MaGOs92 je n'arrive pas à faire marcher la fonctionnalité en local chez moi.
Je teste la fonction window.getCommunesAround(20), elle est bien présente mais renvoie toujours []. Et puis je ne vois aucun appel dans l'onglet réseau à openmaptiles.*
En lisant le code j'ai l'impression que ce dernier commit ajoute nécessairement un poids important à la carte, il va ajouter toute une couche en plus pour chaque recherche locale sur cartes.app, ce qui alourdit quand même beaucoup l'app.
(Par contre je ne connaissais pas cette source vectorielle, elle est cool !)
J'ai une autre idée : simplement ajouter les noms de commune qu'on trouve sur nos tuiles, via un querySourceFeatures comme tu le fais, puis injecter le nom des communes dans la recherche geopf ainsi : https://data.geopf.fr/geocodage/search?index=address&q=19+rue+des+fauvettes+Méral&limit=5&lat=47.9548&lon=-1.05&city=Méral
Comme on le voit dans cette URL, l'ajout de city=Méral comme doublon de Méral déjà présent dans la requête originale ne va pas faire planter l'algo.
Bon par contre en écrivant ces lignes, je me rends compte que contrairement au citycode, on ne peut pas spécifier plusieurs city comme un tableau :/
Globalement, je me rends compte que ce sujet de la recherche localisée non disponible (ou trop partiellement) dans geopf est un vrai problème, car ça cause une regression sur cartes.app, comme on le voit sur ces deux captures.
Avant :
Après :
Globalement, les résultats locaux disparaissent, le mode "ici" ne fonctionne plus.
Cela dit, ta capture ici que je n'arrive pas à reproduire en local, semble avoir réglé le problème, en excluant des résultats de "ici" les résultats dans les communes non présentes dans les 20 km. Cela dit, ça complexifie pas mal notre algo, faut qu'on garde ça en tête.
Au final, je me demande si le plus simple n'est pas d'intégrer dans nos tuiles les codes INSEE des communes. Le poids supplémentaire sera presque négligeable, et je suis sûr qu'on pourra trouver d'autres usages de cette info clé. cc @etienneJr ça semble aisé, non ?
Ça nous éviter ici de faire appel à une autre source de données.
Une autre idée m'est venue à l'esprit. S'il est vrai que beaucoup d'adresses manquent dans OSM, ça me semble beaucoup moins vrai pour les rues.
J'ai l'impression que l'essentiel du travail de ce ticket cible est de résoudre le problème de "il trouve pas mon numéro, donc pas la rue non plus".
D'où l'idée : si Photon ne trouve pas "19 rue des fauvettes", on peut lui refaire une requête locale elle aussi avec "rue des fauvettes". C'est pas trivial, à cause des "bis" et compagnie qui font qu'on ne peut pas simplement juste enlever le numéro.
Ensuite avec cette requête, Photon trouve la rue, nous renvoie le code postal, et l'API adresses n'a qu'à faire la recherche précisément sur ce code postal de cette rue sans numéro.
En résumé : solution pas simple non plus, mais autre stratégie qui me semble viable en tout cas.
@laem wrote in #2365 (comment):
@MaGOs92 je n'arrive pas à faire marcher la fonctionnalité en local chez moi.
Je teste la fonction window.getCommunesAround(20), elle est bien présente mais renvoie toujours []. Et puis je ne vois aucun appel dans l'onglet réseau à openmaptiles.*
En lisant le code j'ai l'impression que ce dernier commit ajoute nécessairement un poids important à la carte, il va ajouter toute une couche en plus pour chaque recherche locale sur cartes.app, ce qui alourdit quand même beaucoup l'app.
(Par contre je ne connaissais pas cette source vectorielle, elle est cool !)
J'ai une autre idée : simplement ajouter les noms de commune qu'on trouve sur nos tuiles, via un querySourceFeatures comme tu le fais, puis injecter le nom des communes dans la recherche geopf ainsi : https://data.geopf.fr/geocodage/search?index=address&q=19+rue+des+fauvettes+Méral&limit=5&lat=47.9548&lon=-1.05&city=Méral
Comme on le voit dans cette URL, l'ajout de city=Méral comme doublon de Méral déjà présent dans la requête originale ne va pas faire planter l'algo.
Bon par contre en écrivant ces lignes, je me rends compte que contrairement au citycode, on ne peut pas spécifier plusieurs city comme un tableau :/
Mince, pardon j'ai poussé trop vite y'avait effectivement un souci : la source découpage administratif se faisait chasser par d'autres changements de style donc maintenant je la ré-injecte si elle a disparue. Normalement le commit 832730c4af devrait fonctionner
Mince, pardon j'ai poussé trop vite y'avait effectivement un souci : la source découpage administratif se faisait chasser par d'autres changements de style donc maintenant je la ré-injecte si elle a disparue. Normalement le commit
832730c4afdevrait fonctionner
OK t'as eu le même problème que moi ici, non ?
@laem wrote in #2365 (comment):
Au final, je me demande si le plus simple n'est pas d'intégrer dans nos tuiles les codes INSEE des communes. Le poids supplémentaire sera presque négligeable, et je suis sûr qu'on pourra trouver d'autres usages de cette info clé. cc @etienneJr ça semble aisé, non ?
Ça nous éviter ici de faire appel à une autre source de données.
Oui ça serait le top si vous pouvez ajouter les codes insee dans vos tuiles vectorielles, on pourrait se passer de cette source supplémentaire et ça ne rajoute pas grand chose en terme quantité de données
Oui ça serait le top si vous pouvez ajouter les codes insee dans vos tuiles vectorielles, on pourrait se passer de cette source supplémentaire et ça ne rajoute pas grand chose en terme quantité de données
Le seul pb que je verrais, c'est de faire une recherche à l'est d'une ville dont le nom (et donc le code INSEE) se trouve hors de l'écran. Mais bon... suffirait de dézoomer :)
@laem wrote in #2365 (comment):
Mince, pardon j'ai poussé trop vite y'avait effectivement un souci : la source découpage administratif se faisait chasser par d'autres changements de style donc maintenant je la ré-injecte si elle a disparue. Normalement le commit
832730c4afdevrait fonctionnerOK t'as eu le même problème que moi ici, non ?
à priori c'était à ce niveau
map.setStyle(styleUrl, { diff: true })
que ça posait problème. Même avec le diff à true, si des sources ne sont pas dans le nouveau style, elles ne sont pas conservées
@laem wrote in #2365 (comment):
Au final, je me demande si le plus simple n'est pas d'intégrer dans nos tuiles les codes INSEE des communes. Le poids supplémentaire sera presque négligeable, et je suis sûr qu'on pourra trouver d'autres usages de cette info clé. cc @etienneJr ça semble aisé, non ?
pour être sûr d'avoir compris : vous voulez qu'on ajoute le code insee sur les points avec le nom de la commune dans le layer place ?
c'est encore d'actualité ou vous avez trouvé une autre solution ?
@laem wrote in #2365 (comment):
Globalement, je me rends compte que ce sujet de la recherche localisée non disponible (ou trop partiellement) dans geopf est un vrai problème, car ça cause une regression sur cartes.app, comme on le voit sur ces deux captures.
on peut leur demander d'ajouter/améliorer cette feature à l'api ?
@etienneJr je pense que les deux sont vraies.
À court terme, ça nous éviterait de charger des données en plus d'avoir juste le code commune dans nos tuiles.
Et deuxièmement, oui je pense qu'Addok devrait mieux gérer la recherche locale.
Mince, je pensais que le code commune était un truc universel du genre "city_id" mais non c'est "ref:INSEE", donc spécifique à la France. Le code postal est un id de moindre qualité car plusieurs CP par ville, mais tant pis, au moins c'est universel dans OMS j'imagine ?
Bon en fait, c'est le bordel : Birmingham a des postal_code pour chaque rue, mais pas de postal_code sur la ville. La plupart des villes d'angleterre non pas "boundary=administration" mais "place". Bref, je sais pas comment dans les tuiles caractériser une ville, lol.
Tant pis, on va se concentrer sur la France pour l'instant.
@etienneJr je te laisse regarder si c'est facile à ajouter ?
@laem wrote in #2365 (comment):
Bon en fait, c'est le bordel : Birmingham a des postal_code pour chaque rue, mais pas de postal_code sur la ville. La plupart des villes d'angleterre non pas "boundary=administration" mais "place". Bref, je sais pas comment dans les tuiles caractériser une ville, lol.
Tant pis, on va se concentrer sur la France pour l'instant.
Oui c'est le cas partout au Royaume-Uni.
Autres cas en France: on a parfois plusieurs communes par code postal.
@laem wrote in #2365 (comment):
@etienneJr je te laisse regarder si c'est facile à ajouter ?
fait ici mais pas testé : cartes/serveur#152
Recompilation des tuiles lancé avec les codes INSEE :)
Erreur inconnue dans le traitement des tuiles. Décidément, c'est pas mon jour. Je regarde en testant sur France localement.
Je pige pas, le process a été tué comme si y avait plus de RAM. Étrange. Soit un pb côté Scaleway, soit un pb dans les changements qu'on a fait à process lua et qui font péter la RAM, ce qui m'étonne beaucoup.
Je retente, vu que pas d'erreur évidente et que ça tourne sur ma machine sur france.osm.pbf sans pb.
@MaGOs92 c'est bon, nos tuiles contiennent maintenant le code INSEE ! Donc ça évite l'ajout d'une source pour faire la même logique que tu as proposé :)
Elles sont inspectables ici.
Désolé, ça a pris du temps, on avait un bug assez rare.
Je comprends si tu trouves que ça fait trop d'itérations que je te demandes, pas de souci dis moi si c'est le cas et je pourrai reprendre ton code pour l'adapter :)
832730c4af
to f6c5fe42b6
Hello @laem, pas de problème. Je viens de mettre à jour la PR pour récupérer les code INSEE depuis les tuiles vectorielles de cartes.app plutôt que celles du découpage administratif. ça semble bien fonctionner sauf sur des très gros zoom où les villes avoisinantes ne doivent pas être chargées dans la carte.
Après le petit souci que je vois c'est que ça peut rajouter un peu de travail de mettre à jour les tuiles quand y'a un changement de COG
Après le petit souci que je vois c'est que ça peut rajouter un peu de travail de mettre à jour les tuiles quand y'a un changement de COG
De mettre à jour OSM du coup. Car si OSM est à jour, nos tuiles le seront, elles sont recalculées chaque samedi sur planet.osm.pbf.
No due date set.
No dependencies set.
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?