-
-
Notifications
You must be signed in to change notification settings - Fork 407
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
feat: add erythritol as a nutrient #7941
Conversation
Kudos, SonarCloud Quality Gate passed! |
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.
Perfect !
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.
Great!
You modified the test but not the algorithm to compute energy: don't you have to subtract erythritol from polyols? Do you intend to do it after?
@@ -726,37 +726,38 @@ sub mmoll_to_unit ($value, $unit) { | |||
'sodium', '!carbohydrates', | |||
'-fiber', '--soluble-fiber-', | |||
'--insoluble-fiber-', '-sugars', | |||
'-added-sugars', '--sucrose-', | |||
'--added-sugars', '--sucrose-', |
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.
--added-sugars
: as I understand it will be displayed as a sub-category of sugars, isn't it? (Which is good I think.)
Does it have an impact on its name in the CSV export?
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.
yes, that makes added sugars a sub category of sugars (as displayed in USDA labels)
I had done it before in fact, but it didn't work because erythritol was not in the nutrients taxonomy:
I'll remove the comment about no corresponding nutrients |
Yes I remember now, thanks a lot! |
This PR adds erythritol as a nutrient, and adds tests for the nutrients to energy coherency test.
Fixes #7837