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,frontend,backend): conserve comments order and sort properties at export #364

Merged
merged 11 commits into from
Feb 7, 2024
Merged

feat(parser,frontend,backend): conserve comments order and sort properties at export #364

merged 11 commits into from
Feb 7, 2024

Conversation

perierc
Copy link
Contributor

@perierc perierc commented Jan 25, 2024 •
edited
Loading

What

  • Currently, all comments are parsed together for an entry node, and rewritten at the beginning of the node at export. This PR modifies the parser to keep the order of the comments during parsing to be able to rewrite them in the same order at export.
    For that, we create a comments field for each commented property/language in each entry node in Neo4J.
    A backend controller (update_node) has also been updated to take these comments into account.
    This PR also disables the display of comments on the frontend, to simplify the app for now.

  • This PR also sorts the properties of an entry node at export to have a deterministic output.

Part of

Resolves #363
Resolves #365

@perierc perierc self-assigned this Jan 25, 2024
@perierc perierc requested a review from a team as a code owner January 25, 2024 13:21
@perierc perierc changed the title feat: conserve comments order feat(parser, frontend): conserve comments order Jan 25, 2024
@github-actions github-actions bot added the tests label Jan 25, 2024
@perierc perierc changed the title feat(parser, frontend): conserve comments order feat(parser, frontend): conserve comments order and sort properties Jan 25, 2024
@perierc perierc changed the title feat(parser, frontend): conserve comments order and sort properties feat(parser, frontend): conserve comments order and sort properties at export Jan 25, 2024
@perierc perierc linked an issue Jan 25, 2024 that may be closed by this pull request
Sort properties of entry nodes at export #365
Closed
@perierc perierc force-pushed the perierc/conserve-comments-order branch from 62ca3f7 to 22f9afc Compare January 31, 2024 10:27
eric-nguyen-cs
eric-nguyen-cs reviewed Jan 31, 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.

A few questions and nits but it seems mostly great!
NB: did you do a few checks on various taxonomy files? I'd bet that there are some very weird edges cases that we have not thought of

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/unparser.py Show resolved Hide resolved
backend/editor/entries.py Show resolved Hide resolved
backend/editor/entries.py Outdated Show resolved Hide resolved
@eric-nguyen-cs
Copy link
Contributor

You can also add comment tests for the parser

alexgarel
alexgarel reviewed Feb 1, 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.

Thanks !

backend/editor/entries.py Show resolved Hide resolved
backend/editor/entries.py Outdated Show resolved Hide resolved
backend/editor/entries.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 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
parser/openfoodfacts_taxonomy_parser/parser/taxonomy_parser.py Show resolved Hide resolved
@eric-nguyen-cs eric-nguyen-cs mentioned this pull request Feb 1, 2024
Closed
@perierc perierc changed the title feat(parser, frontend): conserve comments order and sort properties at export feat(parser,frontend,backend): conserve comments order and sort properties at export Feb 6, 2024
eric-nguyen-cs
eric-nguyen-cs approved these changes Feb 7, 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.

Good

@perierc perierc merged commit 780ad2a into main Feb 7, 2024
7 checks passed
@perierc perierc deleted the perierc/conserve-comments-order branch February 7, 2024 10:41
@openfoodfacts-bot openfoodfacts-bot mentioned this pull request Feb 7, 2024
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

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

Assignees

@perierc perierc

Labels
backend frontend 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.

Sort properties of entry nodes at export Conserve order and place of comments
3 participants
@perierc @eric-nguyen-cs @alexgarel