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

fix: Add multiple labels in backend #101

Merged
merged 28 commits into from
Nov 7, 2022
Merged

fix: Add multiple labels in backend #101

merged 28 commits into from
Nov 7, 2022

Conversation

aadarsh-ram
Copy link
Collaborator

@aadarsh-ram aadarsh-ram commented Oct 6, 2022 •
edited
Loading

What

  • Add multiple labels (to support branch and taxonomy labelling) in parser
  • Add multiple labels in unparser and tests
  • Changed API endpoints accordingly to support different branches and taxonomies
  • Wrapped entries.py in a class for easier use
  • Added import functionality
  • Added export functionality

Related issue(s)

alexgarel
alexgarel requested changes Oct 7, 2022
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.

Cool just small changes and it seems ok to me.

I would like you to add an test for parser.

At some point we should also setup tests, at least for TaxonomyGraph !

parser/openfoodfacts_taxonomy_parser/parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser.py Outdated Show resolved Hide resolved
parser/openfoodfacts_taxonomy_parser/parser.py Outdated Show resolved Hide resolved
parser/tests/integration/test_parse_unparse_integration.py Outdated Show resolved Hide resolved
parser/tests/integration/test_parse_unparse_integration.py Show resolved Hide resolved
backend/editor/entries.py Outdated Show resolved Hide resolved
backend/editor/entries.py Outdated Show resolved Hide resolved
backend/editor/entries.py Outdated Show resolved Hide resolved
@aadarsh-ram aadarsh-ram marked this pull request as ready for review October 8, 2022 16:51
@aadarsh-ram aadarsh-ram requested a review from a team as a code owner October 8, 2022 16:51
alexgarel
alexgarel approved these changes Oct 10, 2022
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.

LGTM but two small suggestions, and a possible refactor but that we can do an a separate PR (as you want).

Note @aadarsh-ram merging this PR will brake the interface (as it still use the old API…). Is it ok ?

alexgarel
alexgarel approved these changes Oct 11, 2022
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.

Ok perfect :-)

@aadarsh-ram aadarsh-ram mentioned this pull request Oct 11, 2022
Merged
@aadarsh-ram
feat: Add new endpoint for fetching root nodes (#104)
cd3593a 
* Add endpoint for root nodes

* Minor fixes
@aadarsh-ram
Copy link
Collaborator Author

aadarsh-ram commented Nov 5, 2022 •
edited
Loading

@alexgarel There seems to be some issue with the docker build in this workflow: https://github.com/openfoodfacts/taxonomy-editor/actions/runs/3399253092/jobs/5652877236#step:3:330

This issue occurs after resolving conflicts in the backend Dockerfile. So, please do take a look at it.

@aadarsh-ram aadarsh-ram mentioned this pull request Nov 6, 2022
Merged
alexgarel
alexgarel approved these changes Nov 7, 2022
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.

LGTM.

@alexgarel alexgarel merged commit a40b9fd into main Nov 7, 2022
@alexgarel alexgarel deleted the multilabelnew branch November 7, 2022 13:31
@aadarsh-ram aadarsh-ram mentioned this pull request Nov 7, 2022
Closed
3 tasks
@openfoodfacts-bot openfoodfacts-bot mentioned this pull request Dec 1, 2022
Merged
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

Assignees

@aadarsh-ram aadarsh-ram

Labels
api backend dependencies 🐳 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.

Change entries.py to support branch and taxonomy name
2 participants
@aadarsh-ram @alexgarel