Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Metrics processing averifier #337

Closed
wants to merge 18 commits into from
Closed

Metrics processing averifier #337

wants to merge 18 commits into from

Conversation

rv2931
Copy link
Collaborator

@rv2931 rv2931 commented Dec 8, 2024 •
edited
Loading

Le merge de la branche initiale metrics-processing nous a paru assez complexe, on a finit par faire un merge de main dans la branche et une PR sur cette branchesur le main de retour... (pas certain que c'était la meilleure solution) mais on arrive à une situation ou la PR présente bien les modifications effectives liées à la branche sans conflits, en attente de validation et intégration main
Evolutions:

  • Ajout d'une table fct_metrics: intégré dans le process ETL, précalcul et stockage d'une partie des stats qui serviront ensuite à faire de l'agragation. La granularité temporelle étant le segment/start/end

ejamet73 and others added 18 commits October 18, 2024 16:39
@ejamet73
Create Alembic migration for fct metrics
9fb7613
@ejamet73
modify Alembic migration for fct metrics
c5a707e
@ejamet73
création de fct_metrics, ajout des colonnes mmsi & ship_name, premier…
211b1a0 
… remplissage de la table lors de l'execution de create_updateexcursions_segments.py
@ejamet73
update create_update_excursion.py
366a90c
@ejamet73
Sur la branche metrics-processing
ac2b8c4 
 Modifications qui seront validées :
	modifié :         backend/alembic/versions/c32d65d6e6fd_create_fct_metrics.py
	modifié :         backend/bloom/container.py
	modifié :         backend/bloom/domain/metrics_new.py
	modifié :         backend/bloom/infra/database/sql_model.py
	modifié :         backend/bloom/infra/repositories/repository_metrics.py
	modifié :         backend/bloom/tasks/create_update_excursions_segments.py
@ejamet73
Create Alembic migration for fct metrics
9f1dc3c
@ejamet73
modify Alembic migration for fct metrics
c3fdf8e
@ejamet73
création de fct_metrics, ajout des colonnes mmsi & ship_name, premier…
f68e4dc 
… remplissage de la table lors de l'execution de create_updateexcursions_segments.py
@ejamet73
update create_update_excursion.py
e9a3ebe
@ejamet73
Sur la branche metrics-processing
11d7066 
 Modifications qui seront validées :
	modifié :         backend/alembic/versions/c32d65d6e6fd_create_fct_metrics.py
	modifié :         backend/bloom/container.py
	modifié :         backend/bloom/domain/metrics_new.py
	modifié :         backend/bloom/infra/database/sql_model.py
	modifié :         backend/bloom/infra/repositories/repository_metrics.py
	modifié :         backend/bloom/tasks/create_update_excursions_segments.py
@ejamet73
update create_update_excursons_segment
010422e
@ejamet73
creation de la table fct_metrics
89d65d7
@ejamet73
Merge remote-tracking branch 'origin/metrics-processing' into metrics…
7a8b9ac 
…-processing
@ejamet73
Changements: Domain/Dataframe, zone_name/mpa_name
2781795
@ejamet73
ajout de zone_sub_category dans fct_metrics
acac647
@ejamet73 @rv2931
update de total_duration dans fct_metrics
6dd77f0
@rv2931
Merge branch 'metrics-processing_before'
15d1905
@marthevienne @rv2931
change down_revision to d9d279892b5c in c32d65d6e6fd
eaaef69
@rv2931
Copy link
Collaborator Author

rv2931 commented Dec 9, 2024

@ejamet73
@marthevienne vient de remonter qu'il manque le champ zone_category en plus de zone_sub_category

@ejamet73
Copy link
Collaborator

@ejamet73 @marthevienne vient de remonter qu'il manque le champ zone_category en plus de zone_sub_category

l'info est contenue dans 'type'

@ejamet73
Copy link
Collaborator

@ejamet73 @marthevienne vient de remonter qu'il manque le champ zone_category en plus de zone_sub_category

l'info est contenue dans 'type'

dans type il y a soit 'DEFAULT_AIS', 'in_amp', 'Fishing coastal waters (6-12 NM)', 'in_zone_with_no_fishing_rights', 'in_territorial_water' . j'ai appelé ça 'type' car on a 'default_ais' et 'in_zone_with_no_fishing_rights' ce qui ne sont pas des zone_category

@rv2931
Copy link
Collaborator Author

rv2931 commented Dec 10, 2024

Pas certain qu'on parle de la même chose
C'est la catégorie de la zone qu'il semble manquer, pas le type
En fait chaque zone a aussi un couple catégorie/subcategories (pas que amp) et je pense que c'est bien d'avoir les deux pour faire des filtres
Enfin c'est de ce que je comprends, à vérifier

@rv2931
Copy link
Collaborator Author

rv2931 commented Dec 10, 2024

Même si je suis d'accord qu'on retrouve une partie de l'info dans type, pour tout ce qui est filtrage/dataviz ça serait pas mal d'avoir la catégorie/sub_categorie en clair.
Et le cas où on rajouterai une catégorie de zone (genre 'white zone' ou autre qui n'est pas prévue dans ta colonne type, celles-ci seraient identifiée comment ? Est-ce ce que cela nécessite une modification du code pour prendre en compte une nouvelle catégorie ?

@ejamet73
Copy link
Collaborator

Même si je suis d'accord qu'on retrouve une partie de l'info dans type, pour tout ce qui est filtrage/dataviz ça serait pas mal d'avoir la catégorie/sub_categorie en clair. Et le cas où on rajouterai une catégorie de zone (genre 'white zone' ou autre qui n'est pas prévue dans ta colonne type, celles-ci seraient identifiée comment ? Est-ce ce que cela nécessite une modification du code pour prendre en compte une nouvelle catégorie ?

white zone Ça implique une modification du code oui. Je peux faire une colonne zone.category, mais il faut me dire sous quelle forme vous voulez integrer les infos défauts d'ais et l'info des zones sans droits de pêche. Si je fais une colonne zone.category l'ajout de white zone ne nécessitera pas de changement de code

@rv2931
Copy link
Collaborator Author

rv2931 commented Dec 10, 2024

Je pense qu'on peut tout a fait garder ta colonne 'type'
Après c'est plutot a mon avis rajouter un type 'in_zone' pour toutes les autres zones qui ne sont pas déjà traitée spécifiquement 'amp' Territoriales...
Comme ça on aura quand meme les stats sur des zones qui sont pas forcément traitées aujourd'hui et si on veut leur mettre un type par la suite il suffirait de traiter l'historique pour changer le type 'in_zone' par un truc plus spécifique sur la base de la valeur de la colonne 'category' et/ou sub_category justement

@marthevienne
Copy link
Collaborator

Ok pour type, on laisse tel quel pour l'instant. On merge ?

@rv2931 rv2931 mentioned this pull request Dec 10, 2024
Merged
@rv2931
Copy link
Collaborator Author

rv2931 commented Dec 10, 2024

Cette PR a été très compliquée à gérer, @ejamet73 j'ai finalement réussi à faire une branche poprement rebasée qui par bien du main actuel et qui contient exactement les même modifications
Donc pour avoir un arbre un peu plus propre je propose d'intégrer cette PR #340
#340

@rv2931
Copy link
Collaborator Author

rv2931 commented Dec 10, 2024

Du coup je propose de merger la #340, fermer cette PR #337
Si évolution il y a on passe via de nouveaux issues

@rv2931 rv2931 closed this Dec 10, 2024
@marthevienne marthevienne mentioned this pull request Dec 11, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@rv2931 rv2931

@ejamet73 ejamet73

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@rv2931 @ejamet73 @marthevienne