-
Notifications
You must be signed in to change notification settings - Fork 51
Analyst/lounes_pfas_udi_annuel #148
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.
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
Conversation
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (5)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci pour le travail :)
J'ai plusieurs remarques sur les choix de syntaxe qui sont différents de cvm. Mais rien de gênant.
La table finale ne respecte pas le modèle de données (plusieurs colonnes "ratio_depassements_XX"
et "nb_depassements_XX"
au lieu de "résultats"
ou d'un seul pourcentage pour la carte ).
==> A clarifier avec GF
dbt_/models/intermediate/pfas/int__resultats_pfas_udi_annuel.sql
Outdated
Show resolved
Hide resolved
dbt_/models/intermediate/pfas/int__resultats_pfas_udi_annuel.sql
Outdated
Show resolved
Hide resolved
dbt_/models/intermediate/pfas/int__resultats_pfas_udi_annuel.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Merci pour le travail.
Je n'ai pas eu du temps pour regarder en détail mais j'ai vu que tu as mis "la somme des 4 PFAS réglementaires dépasse le seuil de 0.1 µg/L." mais je crois que d'après le brief le seuil est 0.02 µg/L.
Et je pour cette partie
--> Au moins 1 PFAS ≥ valeur sanitaire (fait passer l’affichage total polluant en rouge)
J'ai vu votre discussions avec GF sur le doc brief et je pense qu'on devrait bien différencier (comprendre) les termes "limites réglementaires", "les valeurs sanitaires", "limite de qualité" etc. Je vais essayer de comprendre.
dbt_/models/intermediate/pfas/int__resultats_pfas_udi_annuel.sql
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ajouter le fichier de test ici aussi :)
Merci pour la review @20k-P et @githubzey ! Modifications faites et fichier de test ajouté 🙌🏻 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
J'ai l'impression que c'est ok
[OUTDATED]
Bilan annuel par UDI pour les résultats PFAS.
Je trouve la query finale un peu longue, mais je n'ai pas trouvé de manière plus simple de la faire. J'ai agrégé les infos en deux étapes :
1/ une fois par 'prelevement / udi / année' pour calculer les valeurs de chaque substance / groupe de substances
2/ une deuxième fois par 'udi / année' pour calculer les nb de prélèvements, dépassements et les ratios
Les conditions finales sont à valider avec GF. Quels sont les ratios à surveiller ? Je me suis plus ou moins basé sur ce qu'on a fait pour les derniers résultats...
[UPDATE 13/04]
Mise à jour de la query pour suivre le brief GF.
On a les colonnes de résultat suivantes :
ratio_depassements_limite_reg
resultat_limite_sanitaire