-
Notifications
You must be signed in to change notification settings - Fork 116
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-1417060: Pylance throws errors when using copy_options with copy_into_location method #1575
SNOW-1417060: Pylance throws errors when using copy_options with copy_into_location method #1575
Conversation
class CopyOptions(TypedDict, total=False): | ||
overwrite: bool | ||
single: bool | ||
max_file_size: float | ||
include_query_id: bool | ||
detailed_output: bool |
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.
@overload | ||
def copy_into_location( | ||
self, | ||
location: str, | ||
*, | ||
partition_by: Optional[ColumnOrSqlExpr] = None, | ||
file_format_name: Optional[str] = None, | ||
file_format_type: Optional[str] = None, | ||
format_type_options: Optional[Dict[str, str]] = None, | ||
header: bool = False, | ||
statement_params: Optional[Dict[str, str]] = None, | ||
block: bool = True, | ||
**copy_options: Unpack[CopyOptions], | ||
) -> Union[List[Row], AsyncJob]: | ||
... # pragma: no cover |
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.
The type hint of a function definition without overload did not work with csv method or json method, and a pylancen error occurred, so I also defined it with overload.
@@ -834,3 +835,10 @@ def type_string_to_type_object(type_str: str) -> DataType: | |||
ColumnOrSqlExpr = Union["snowflake.snowpark.column.Column", str] | |||
LiteralType = Union[VALID_PYTHON_TYPES_FOR_LITERAL_VALUE] | |||
ColumnOrLiteral = Union["snowflake.snowpark.column.Column", LiteralType] | |||
|
|||
class CopyOptions(TypedDict, total=False): |
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.
TypedDict
and total arguments are supported starting with Python 3.8.
https://docs.python.org/3.8/library/typing.html?highlight=typeddict#typing.TypedDict
@@ -4,6 +4,7 @@ | |||
|
|||
import sys | |||
from typing import Any, Dict, List, Literal, Optional, Union, overload | |||
from typing_extensions import Unpack |
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.
Unpack
is supported starting with typing_extensions==4.1.0
.
https://github.com/python/typing_extensions/blob/e3359a92257b7abcbb5d24a1f3a81c1b007defbc/CHANGELOG#L1-L4
I have read the CLA Document and I hereby sign the CLA |
Are you looking at the PR? |
38fc8e6
to
625519f
Compare
625519f
to
85cb3c7
Compare
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1417060
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Instead of using Dict[str, Any] for the copy_options argument, I have switched to a TypedDict to enforce stricter type checking and provide better clarity on the expected options.