-
-
Notifications
You must be signed in to change notification settings - Fork 22
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: add error logs to parser #37
Conversation
Not adding to tags_ids if it's already in it by another normalized synonym
Remove end-line comma
Used black
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.
Really good, just added one comment, but I will fix it myself :-)
correctly_written.match(property_name) | ||
and correctly_written.match(lc) | ||
): | ||
raise Exception |
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.
You have to instanciate the Exception when you throw it.
It's not a good pattern to use an Exception an empty expect, because you may hide some other exception. Here you just have to use the if / then / else
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.
I see it's to unify handling of the ValueError in split, but I still think it's not a good idea ;-)
@BryanH01 thinking how we will use the parser, I think a good addition to your job, would be to collect errors instead of simply log them and add them in the neo4j database in a special parsing_errors entry. |
⬆️ made it an issue : #57 |
What