-
Notifications
You must be signed in to change notification settings - Fork 3
Ensure semantic information is all correctly stored in the metadata #465
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
Conversation
Co-authored-by: john-sanchez31 <[email protected]>
john-sanchez31
left a comment
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.
Looks good! Thanks Kian
hadia206
left a comment
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.
Looks good.
Thanks Kian!
pydough/metadata/parse.py
Outdated
| None, | ||
| None, | ||
| {}, | ||
| extra_info, |
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.
Should this variable be named extra_semantic_info instead of extra_info? To keep consistence with the other names that use the same name as the JSON file and class property.
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.
Sure why not
Adds extra tests to ensure the extra semantic information fields (sample values, description, extra semantic info, synonyms, verified pydough analysis, additional definitions) are all present in graph/collection/property metadata. Modifies the parsing to account for cases where the information is not already being passed along, and modifies the TPC-H graph to add some extra semantic information of this variety.