feat: set max percent of sugar and salt ingredients based on nutrition facts #9276
Conversation
github-actions
bot
added
🥗 Ingredients
🧽 Data quality
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #9276 +/- ##
==========================================
+ Coverage 47.95% 48.04% +0.09%
==========================================
Files 65 65
Lines 20277 20304 +27
Branches 4914 4922 +8
==========================================
+ Hits 9724 9756 +32
+ Misses 9301 9298 -3
+ Partials 1252 1250 -2 ☔ View full report in Codecov by Sentry. |
|
I added a new test set with 1000 products with some specified ingredients percent. Before this PR: Test set world-1000-some-specified With this PR: Test set world-1000-some-specified |
lib/ProductOpener/Ingredients.pm
Outdated
|
|
||
| =cut | ||
|
|
||
| sub add_percent_max_for_salt_and_sugar_ingredients_from_nutrition_facts ($product_ref) { |
There was a problem hiding this comment.
Why not add_percent_max_for_ingredients_from_nutrition_facts as we could add more in the future ?
lib/ProductOpener/Ingredients.pm
Outdated
| unshift @ingredients, @{$ingredient_ref->{ingredients}}; | ||
| } | ||
|
|
||
| foreach my $ingredient_max_value_ref (@ingredient_max_values) { |
There was a problem hiding this comment.
We already iterated above ! Why do we need to iterate twice ?
(I prefer if we keep this inner loop, that said).
There was a problem hiding this comment.
ah good point, initially I had it in the inner loop but I wanted to move it to the outer loop, and I forgot to remove the inner loop.. :(
we can keep the inner loop only
lib/ProductOpener/Ingredients.pm
Outdated
| foreach my $ingredient_max_value_ref (@ingredient_max_values) { | ||
| my $value = $ingredient_max_value_ref->{value}; | ||
| if (is_a("ingredients", $ingredient_ref->{id}, $ingredient_max_value_ref->{ingredientid})) { | ||
| if (not defined $ingredient_ref->{percent_max}) { |
There was a problem hiding this comment.
why don't we do it if percent_max is above current max ?
| if (not defined $ingredient_ref->{percent_max}) { | |
| if ((not defined $ingredient_ref->{percent_max}) or ($value < $ingredient_ref->{percent_max})) { |
There was a problem hiding this comment.
We could, it was just to be safe if the sugar or salt is not 100% sugar or salt.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Shan't we do a pass after that to eventually re-order the list of ingredients (or sub-ingredients) ?
There was a problem hiding this comment.
The ingredients are listed by quantity order, we keep them in this order.
Co-authored-by: Alex Garel <[email protected]>
Co-authored-by: Alex Garel <[email protected]>
Co-authored-by: Alex Garel <[email protected]>
|
Kudos, SonarCloud Quality Gate passed!
|
|
@stephanegigandet applied changes, @alexgarel can we merge ? |
A small improvement to ingredients percent estimation, made to test the new https://github.com/openfoodfacts/recipe-estimator-metrics framework.
Corresponding changes: openfoodfacts/recipe-estimator-metrics@32cb058
e.g. test summary differences on a small test set of 62 products:
test-sets/results/product_opener/fr-62-products-10-specified-ingredients/results_summary.json