-
Notifications
You must be signed in to change notification settings - Fork 112
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
SNOW-1791994 IR: Generate typed protobuf Python code #2593
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -91,6 +91,7 @@ repos: | |
- id: mypy | ||
files: > | ||
(?x)^( | ||
src/snowflake/snowpark/_internal/ast/.*\.py| | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. cover all ast code in mypy type check |
||
src/snowflake/snowpark/modin/pandas/indexing.py| | ||
src/snowflake/snowpark/modin/plugin/_internal/.*\.py| | ||
src/snowflake/snowpark/modin/pandas/snow_partition_iterator.py| | ||
|
@@ -103,9 +104,12 @@ repos: | |
- --disallow-incomplete-defs | ||
additional_dependencies: | ||
- types-requests | ||
- types-python-dateutil | ||
- types-urllib3 | ||
- types-setuptools | ||
- types-pyOpenSSL | ||
- types-setuptools | ||
- mypy-protobuf | ||
- typed-ast | ||
- pytest | ||
- numpy |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -57,6 +57,7 @@ | |
"pytest-assume", # sql counter check | ||
"decorator", # sql counter check | ||
"protoc-wheel-0==21.1", # Protocol buffer compiler, for Snowpark IR | ||
"mypy-protobuf", # used in generating typed Python code from protobuf for Snowpark IR | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I assume this has no licensing or security concerns, either. Should this require a specific minimum version? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, this is development requirements so should be fine. |
||
"lxml", # used in read_xml tests | ||
] | ||
|
||
|
@@ -81,7 +82,7 @@ | |
|
||
if protoc is None: | ||
sys.stderr.write( | ||
"protoc is not installed nor found. Please install the binary package, e.g., `pip install protoc-wheel-0==21.1`\n" | ||
"protoc is not installed nor found. Please install the binary package, e.g., `pip install protoc-wheel-0==21.1 mypy-protobuf`\n" | ||
) | ||
sys.exit(-1) | ||
|
||
|
@@ -115,7 +116,7 @@ def generate_proto(source): | |
protoc, | ||
f"--proto_path={proto_dir}", | ||
f"--python_out={output_dir}", | ||
f"--pyi_out={output_dir}", | ||
f"--mypy_out={output_dir}", | ||
source, | ||
] | ||
if subprocess.call(protoc_command) != 0: | ||
|
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.
just reuse the same code in tox