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: Decouple parsing the taxonomy and writing the taxonomy to the database #317

Merged
merged 12 commits into from
Jan 17, 2024
Merged

feat: Decouple parsing the taxonomy and writing the taxonomy to the database #317

merged 12 commits into from
Jan 17, 2024

Conversation

eric-nguyen-cs
Copy link
Contributor

@eric-nguyen-cs eric-nguyen-cs commented Dec 21, 2023 •
edited
Loading

What

  • The goal of this PR is to decouple the action of parsing the taxonomy .txt file into Python objects and the action of writing this data into the Neo4J database

Note

This PR is quite big, so to simplify the review, you can go through commit by commit (I tried to make the commits quite small and atomic)

Fixes bug(s)

#88

Part of

  • This work is the precursor of the parsing performance work. See next PR

@eric-nguyen-cs eric-nguyen-cs requested a review from a team as a code owner December 21, 2023 12:53
@eric-nguyen-cs eric-nguyen-cs changed the title [FEAT] Decouple parsing the taxonomy and writing the taxonomy to the database feat: Decouple parsing the taxonomy and writing the taxonomy to the database Dec 21, 2023
@eric-nguyen-cs eric-nguyen-cs mentioned this pull request Dec 21, 2023
Merged
Piv94165
Piv94165 approved these changes Dec 21, 2023
View reviewed changes
Copy link
Contributor

@Piv94165 Piv94165 left a comment

Choose a reason for hiding this comment

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

Indeed the clarity in this version is a significant improvement !

alexgarel
alexgarel approved these changes Jan 9, 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.

Kudos !

I have a proposal to change the composition pattern a bit, but it's all good.

parser/openfoodfacts_taxonomy_parser/parser/parser.py Show resolved Hide resolved
@eric-nguyen-cs eric-nguyen-cs merged commit 71b34be into main Jan 17, 2024
10 checks passed
@eric-nguyen-cs eric-nguyen-cs deleted the ericn/decouple-parser-and-db-writer branch January 17, 2024 11:50
@openfoodfacts-bot openfoodfacts-bot mentioned this pull request Jan 17, 2024
Merged
perierc pushed a commit that referenced this pull request Jan 17, 2024
@eric-nguyen-cs @perierc
feat: Decouple parsing the taxonomy and writing the taxonomy to the d…
a2ba34d 
…atabase (#317)

* refactor: mark private function with _

* refactor(parser): add type annotations and clean up code

* chore: use context manager to close session in tests

* chore: update neo4j and Makefile

* refactor: create parser specific directory

* refactor: start taxonomy_parser by copying parser file

* refactor: move logger to separate file

* refactor: remove unnecessary code for taxonomy parser

* feat: update TaxonomyParser to return taxonomy class

* feat: update parser to use taxonomy parser

* chore: update tests for new taxonomy parser
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@alexgarel alexgarel alexgarel approved these changes

@Piv94165 Piv94165 Piv94165 approved these changes

Assignees

@eric-nguyen-cs eric-nguyen-cs

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

Successfully merging this pull request may close these issues.
3 participants
@eric-nguyen-cs @alexgarel @Piv94165