-
-
Notifications
You must be signed in to change notification settings - Fork 404
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
fix: fix_dq_facet_all_values_identical #9722
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #9722 +/- ##
=======================================
Coverage 49.54% 49.54%
=======================================
Files 67 67
Lines 20650 20650
Branches 4980 4980
=======================================
Hits 10231 10231
Misses 9131 9131
Partials 1288 1288 ☔ View full report in Codecov by Sentry. |
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.
In addition, I think you should add a sort in there:
foreach my $nid (keys %{$product_ref->{nutriments}}) {
->
foreach my $nid (sort keys %{$product_ref->{nutriments}}) {
So that we always go in the same order.
lib/ProductOpener/DataQualityFood.pm
Outdated
($nutriments_values_occurences_max_value == scalar @major_nutriments_values) | ||
( | ||
$nutriments_values_occurences_max_value == scalar @major_nutriments_values | ||
and (@major_nutriments_values[0] > 1) |
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.
and (@major_nutriments_values[0] > 1) | |
and ($major_nutriments_values[0] > 1) |
This currently causes a warning in the main branch:
Scalar value @major_nutriments_values[0] better written as $major_nutriments_values[0] at /opt/product-opener/lib/ProductOpener/DataQualityFood.pm line 1171.
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.
done.
Quality Gate passedKudos, no new issues were introduced! 0 New issues |
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.
Thanks!
* fix_dq_facet_all_values_identical * apply sugestions
What
Bug reported on Slack
Sometimes the unit test 'all identical values and above 1 in the nutrition table 2' in dataqualityfood.t fails.
It happens because the array "@major_nutriments_values" (see Ingredients.pm) can have values in random order. Sometimes the first value is "0.8" and since we have the condition "@major_nutriments_values[0] > 1" facet is not raised. And test fails.
See the code for suggested solution