Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(parser): handle duplicate ID errors during parsing #408

Merged
merged 6 commits into from
Feb 14, 2024
Merged

feat(parser): handle duplicate ID errors during parsing #408

merged 6 commits into from
Feb 14, 2024

Conversation

perierc
Copy link
Contributor

@perierc perierc commented Feb 14, 2024

What

  • This PR merges the duplicate nodes (having the same ID) during parsing.
  • When a node with an already existing ID is harvested, we yield it, but we don't link it to any other node (no PreviousLink is related to the duplicate)
  • Added a new step to remove duplicate child links (due to the fact that duplicate nodes are yielded)
  • Added a step to remove duplicate entry nodes: translations are merged (union), properties are overwritten with the latest values found, comments are added.

Screenshot

The new error message for duplicate IDs looks like this:

image

Part of

@perierc perierc requested a review from a team as a code owner February 14, 2024 12:38
alexgarel
alexgarel reviewed Feb 14, 2024
View reviewed changes
Copy link
Member

@alexgarel alexgarel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Really cool.

Some cosmetic changes proposed.

@eric-nguyen-cs do you remember a good taxonomy to test this ?

parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Show resolved Hide resolved
@perierc
Copy link
Contributor Author

perierc commented Feb 14, 2024

Really cool.

Some cosmetic changes proposed.

@eric-nguyen-cs do you remember a good taxonomy to test this ?

FYI I've tried with the Ingredients taxonomy (with 39 duplicate id errors) and I've verified manually that the first 10 errors were correctly merged, comparing with the original file, and it was all good

@eric-nguyen-cs
Copy link
Contributor

A few examples of the end result:
Screenshot 2024-02-14 at 16 15 30

Screenshot 2024-02-14 at 16 19 03

eric-nguyen-cs
eric-nguyen-cs approved these changes Feb 14, 2024
View reviewed changes
Copy link
Contributor

@eric-nguyen-cs eric-nguyen-cs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Show resolved Hide resolved
@perierc perierc merged commit e5720bf into main Feb 14, 2024
7 checks passed
@perierc perierc deleted the perierc/handle-duplicate-id branch February 14, 2024 20:48
@openfoodfacts-bot openfoodfacts-bot mentioned this pull request Feb 14, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexgarel alexgarel alexgarel left review comments

@eric-nguyen-cs eric-nguyen-cs eric-nguyen-cs approved these changes

Assignees

@perierc perierc

Labels
frontend parser tests
Projects
No open projects
🧬 Taxonomy editor - gsoc 2022
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

handle duplicate id while parsing
3 participants
@perierc @eric-nguyen-cs @alexgarel