Test(feat): add fake city #93
Conversation
| self.assertTrue(df.tiles_generated.values) | ||
| self.assertEqual(Tile.objects.count(), 587) | ||
| self.assertEqual(Tile.objects.count(), 25) |
There was a problem hiding this comment.
Merci d’avoir écrit ce test.
Ça met en évidence mon incompréhesion dans la manière sont générées les tuiles.
En l’occurrence, on cherche à recouvrir un carré de 400 m avec un quadrillage de côté 200m. Si les coordonnées étaient parfaitement alignées sur la grille, on aurait seulement besoin de quatre tuiles.
mais comme ce n’est pas aligné dans notre cas, on a besoin de 3 tuiles en largeur, 3 tuiles en hauteur donc 3x3 = 9 tuiles en tout.
Je ne comprends pas à quel moment on a besoin de 25 tuiles.
Le problème vient de la fonction c02_init_grid.py::create_tiles_for_city où on fait
xmin, ymin, xmax, ymax = city_geom.bounds
xmin -= grid_size
ymin -= grid_size
xmax += grid_size
ymax += grid_size
# Bounds for the generation
xmin, ymin, xmax, ymax = tile_shape_cls.adjust_bounds(
xmin, ymin, xmax, ymax, grid_size, side_length
)Peux-tu éclairer ma lanterne @ludovicdmt ?
There was a problem hiding this comment.
Les villes ont parfois des géométries bizarres qui créent des problèmes de bord aux frontières quand je crées pile le bon nombre. Ca fait des tuiles manquantes donc je prends de la marge un peu partout pour l'éviter.
C'est pas opti mais j'avoue que j'ai pas poussé le truc parce que c'est déjà beaucoup mieux que ce que j'ai récupéré au départ où l'on prennait juste un rectangle englobant de la ville donc encore beaucoup plus de tuiles en trop.
Il y dans c02_init_grid une étape finale qui supprime toutes les tuiles en doublon et celles qui sont hors de l'union de toutes les villes.
Néanmoins j'ai refait une passe pour limiter la création de tuiles en trop parce que je suis d'accord que c'était un peu abusé !
back/iarbre_data/tests/test_views.py
Outdated
There was a problem hiding this comment.
Je ne vois pas le rapport avec le reste de la PR.
There was a problem hiding this comment.
En l'état si tu as déjà des .mvt générés par ailleurs, tu ne peux pas run les tests.
Ca a bloqué Maxime cette aprèm et moi aussi là parce que j'en avais donc j'ai corrigé.
J'ai rajouté ce point dans la description de la PR.
There was a problem hiding this comment.
Je comprenais le point, mais je ne comprends toujours pas pourquoi cela devait être ajouté dans cette MR. Globalement, je suis pour qu’on n'ajoute pas de nouvelles specs pour une MR relue, sauf si demandé par la revue.
There was a problem hiding this comment.
J'ai ajouté dans cette MR en me disant que cette MR était un run pour améliorer les tests de manière globale.
Cela étant dit, de rajouter fait trainer le merge. Je suis d'accord que c'était une mauvaise pratique de ma part de rajouter en cours de route et ça n'est pas à reproduire sur les prochaines.
fake-cityun carré de 400m pour des tests plus simples à comprendrefile_data