Conversation
| for i, threshold in enumerate(PLANTABILITY_THRESHOLDS): | ||
| if value < threshold: | ||
| return i * 2 | ||
| return 10 |
There was a problem hiding this comment.
Normal la suppression de cette ligne ça retourne None maintenant
There was a problem hiding this comment.
Est-ce que tu ne veux pas plutôt lever une exception ici ? Normalement ça ne devrait pas se produire non ?
(ma crainte est que le None ne casse rien ici mais casse un truc qqs fonctions plus loin et que ce soit donc difficile à déboguer).
Marc-AntoineA
left a comment
Marc-AntoineA
left a comment
There was a problem hiding this comment.
Quelques recommendations pour améliorer la lisibilité du code mais j’ai l’impression ok pour moi une fois que tu auras fait le tri dans ce mes commentaires.
There was a problem hiding this comment.
Ok, j’ai il faut que tu récupères le submodule, la première, fois, faire :
git submodule update --init --recursive
| Default = "Default" | ||
| } | ||
|
|
||
| export const MIN_ZOOM = 12 |
There was a problem hiding this comment.
Bonus : Pas moyen d'avoir ces constantes à un seul endroit (pour back et front). Tu peux par ex. lire un fichier de config dans vite.
| @@ -37,17 +39,17 @@ def normalize_plantability(value: float) -> float: | |||
| The normalized plantability index. | |||
| """ | |||
| if value < -5: | |||
There was a problem hiding this comment.
Les thresholds sont déjà dans une constante (PLANTABILITY_THRESHOLDS) tu peux donc remplacer cette fonction par une boucle for.
Et je suis d'avis que cette fonction soit bougée dans plantability/constants.py à côté des constantes.
N’est-ce pas la fonction score_thresholding exactement ?
| for i, threshold in enumerate(PLANTABILITY_THRESHOLDS): | ||
| if value < threshold: | ||
| return i * 2 | ||
| return 10 |
There was a problem hiding this comment.
Est-ce que tu ne veux pas plutôt lever une exception ici ? Normalement ça ne devrait pas se produire non ?
(ma crainte est que le None ne casse rien ici mais casse un truc qqs fonctions plus loin et que ce soit donc difficile à déboguer).
| from plantability.constants import PLANTABILITY_NORMALIZED | ||
|
|
||
|
|
||
| def get_tile_color(normalized_indice: float) -> str: |
back/api/utils/mvt_generator.py
Outdated
| if len(batch_df_clipped) == 0: | ||
| continue | ||
|
|
||
| df_grid_clipped = self._make_grid_aggregate( |
There was a problem hiding this comment.
Ici tu ne vérifies pas que le zoom <= 15, c’est ok car en effet zoom < 13 plus haut, sauf que c’est visiblement pour des raisons différentes et à l’avenir, on pourrait, sur une machine ultra puissante changer la condition du if là haut if zoom <= 20 and side_length < 10 et alors on ferait quand même l’aggrégat
back/api/utils/mvt_generator.py
Outdated
| ) | ||
| if not batch_queryset.exists(): | ||
| continue | ||
| batch_df = load_geodataframe_from_db( |
There was a problem hiding this comment.
J’ai vraiment l’impression qu’on pourrait mutualiser ce code (lignes 255 à 260) avec les lignes 275 à 280.
- le
iddans leload_geodataframe_from_dbne me semble pas utile (?). - le
clipdiffère entre les deux blocs et pourrait être l’argument de la fonction (batch_clip_gdfvsclip_mvt_gdf)
There was a problem hiding this comment.
- Le
idest utilisé dans le_create_mvt_features(ligne 354)
back/api/utils/mvt_generator.py
Outdated
| grid = self.create_grid(df_clipped, grid_size) | ||
| grid = gpd.clip(grid, df_clipped) | ||
| spatial_join = gpd.sjoin(df_clipped, grid, how="left", predicate="intersects") | ||
| aggregated = ( |
back/api/utils/mvt_generator.py
Outdated
| return df_clipped | ||
|
|
||
| @staticmethod | ||
| def _create_mvt_features( |
There was a problem hiding this comment.
J’ai l’impression que ça ne fonctionne que pour la plantability, l’indiquer dans le nom ?
back/api/utils/mvt_generator.py
Outdated
|
|
||
| @staticmethod | ||
| def map_to_discrete_value(x): | ||
| """Map average value of normalized plantability to normalized plantability set""" |
There was a problem hiding this comment.
Ça peut être utilisé pour autre chose que "l’average value" non ?
3 participants
On ne pouvait pas générer des MVTs avec des tuiles 5x5 pour cause d'OOM.
J'ai donc un peu fait des modifs pour :
load_geodataframe_from_db)