-
Notifications
You must be signed in to change notification settings - Fork 119
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-1304211: Pylance throws errors when using copy_options with copy_into_location method #1372
SNOW-1304211: Pylance throws errors when using copy_options with copy_into_location method #1372
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
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.
Thanks for your contribution!
The changes LGTM, I suggested 2 additional type hint fixes.
@@ -240,7 +240,7 @@ def copy_into_location( | |||
header: bool = False, | |||
statement_params: Optional[Dict[str, str]] = None, | |||
block: bool = True, |
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.
block: bool = True, | |
block: Literal[True], |
@@ -256,7 +256,7 @@ def copy_into_location( | |||
header: bool = False, | |||
statement_params: Optional[Dict[str, str]] = None, | |||
block: bool = 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.
block: bool = False, | |
block: Literal[False], |
@sfc-gh-stan I have added a default value to the blocks keyword argument. This is because keyword arguments are optional, and not providing a value would result in an error. |
Tests kicked off by #1377, will merge after merge gates pass. |
Please answer these questions before submitting your pull requests. Thanks!
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1304211: Pylance throws errors when using
copy_options
withcopy_into_location
method #1363Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
src/snowflake/snowpark/dataframe_writer.py
: Thecopy_into_location
method was updated in three instances. Thecopy_options
parameter was changed fromOptional[str]
toOptional[Dict[str, Any]]
. This change allows for a wider range of types to be accepted as copy options, providing more flexibility when calling the method. [1] [2] [3]