-
-
Notifications
You must be signed in to change notification settings - Fork 68
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: Added robotoff nutrient extraction #1026
base: master
Are you sure you want to change the base?
Conversation
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.
Thank you @PrimaelQuemerais for your PR!
Please have a look at my comments.
@@ -1,6 +1,7 @@ | |||
import 'dart:convert'; | |||
|
|||
import 'package:http/http.dart'; | |||
import 'package:openfoodfacts/src/model/robotoff_nutrient_extraction.dart'; |
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.
import 'package:openfoodfacts/src/model/robotoff_nutrient_extraction.dart'; | |
import 'model/robotoff_nutrient_extraction.dart'; |
/// Get nutrient extraction insights for a given barcode | ||
/// cf. https://robotoff.openfoodfacts.org/api/v1/insights | ||
static Future<RobotoffNutrientExtractionResult> getNutrientExtraction( | ||
String barcode, { |
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.
String barcode, { | |
final String barcode, { |
@@ -1,4 +1,5 @@ | |||
import 'package:openfoodfacts/openfoodfacts.dart'; | |||
import 'package:openfoodfacts/src/model/robotoff_nutrient_extraction.dart'; |
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.
import 'package:openfoodfacts/src/model/robotoff_nutrient_extraction.dart'; |
That file should be included in openfoodfacts.dart
.
@@ -1,7 +1,5 @@ | |||
// GENERATED CODE - DO NOT MODIFY BY HAND | |||
|
|||
// ignore_for_file: deprecated_member_use_from_same_package |
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.
Please keep it there, we need it.
@JsonSerializable() | ||
class RobotoffNutrientExtractionResult extends JsonObject { |
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.
Generally speaking, I would prefer one file per class (I'm from Java) and an explicit Robotoff
prefix for all class names.
Not the most important part of my review, though.
final Map<String, List<NutrientEntity>>? entities; | ||
final Map<String, NutrientEntity>? nutrients; |
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.
That String
key is a vague.
Please consider adding getters with Nutrient
and PerSize
parameters.
expect(result.status, 'found'); | ||
expect(result.insights!, isNotEmpty); | ||
expect(result.insights![0].barcode, barcode); | ||
expect(result.insights![0].data!.nutrients!['fat_100g']!.value, '0.5'); |
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.
Nutrient + PerSize
final String? barcode; | ||
final String? type; | ||
final NutrientDataWrapper? data; | ||
final String? timestamp; |
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.
Is that a date?
@JsonKey(name: 'id') | ||
final String? insightId; | ||
final String? barcode; | ||
final String? type; |
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.
Are there known types?
@JsonKey(name: 'n_votes') | ||
final int? nVotes; | ||
final String? username; | ||
final List<String>? countries; |
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.
Localized countries? Country codes?
What
This PR adds the nutrient extraction Robotoff feature to the package
Part of
#738