-
Notifications
You must be signed in to change notification settings - Fork 70
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
Initial commit to push semantic model generator #1
Initial commit to push semantic model generator #1
Conversation
# semantic-model-generator |
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.
@sfc-gh-pverhoeven Please review
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.
Reviewed the README
and left some comments. PTAL and let me know if you have any questions.
semantic_model_generator/main.py
Outdated
populates them with sample values, and sets placeholders for descriptions and filters. | ||
""" | ||
|
||
# For each columns, decide if it is a TimeDimension, Measure, or Dimension column. |
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.
decide -> assume?
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 think this is ok. wdyt
semantic_model_generator/snowflake_utils/snowflake_connector.py
Outdated
Show resolved
Hide resolved
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.
Looking great!
semantic_model_generator/snowflake_utils/snowflake_connector.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Pieter Verhoeven <[email protected]>
Co-authored-by: Pieter Verhoeven <[email protected]>
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 great! Thanks for putting this together so quickly!
README.md
Outdated
|
||
|
||
```bash | ||
python -m semantic_model_generator.main \ | ||
--fqn_tables "['<your-database-name-1>.<your-schema-name-1>.<your-table-name-1>','<your-database-name-2>.<your-schema-name-2>.<your-table-name-2>']" \ | ||
--fqn_tables "['<your-database-name-1>.<your-schema-name-1>.<your-physical-table-name-1>','<your-database-name-2>.<your-schema-name-2>.<your-physical-table-name-2>']" \ | ||
--semantic_model_name "<a-meaningful-semantic-name-for-your-team>" \ |
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.
WDYT about s/a-meaningful-semantic-name-for-your-team/a-meaningful-semantic-model-name
?
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.
updated!
README.md
Outdated
|
||
In addition, consider adding the following elements to your semantic model: | ||
|
||
1. Logical columns for a given table. |
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 we say, "Logical columns that are expressions over physical columns"? Since all columns in a semantic model are logical columns.
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.
good point - done
The initial commit to move over the relevant files to the
semantic-model-generator
repo.