-
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
Typing #1056
Typing #1056
Conversation
CLA Assistant Lite bot All contributors have signed the CLA ✍️ ✅ |
I have read the CLA Document and I hereby sign the CLA |
Test pr in #1061 |
|
@@ -90,7 +93,10 @@ def convert_result_meta_to_attribute(meta: List[ResultMetadata]) -> List[Attribu | |||
Attribute( | |||
quoted_name, | |||
convert_sf_to_sp_type( | |||
FIELD_ID_TO_NAME[type_value], precision, scale, internal_size | |||
FIELD_ID_TO_NAME[type_value], | |||
precision or 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.
I wonder if the default should be 38.
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.
What happens today if it is None
?
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.
This is only None when the result is not Decimal
type, where the parameter would not be used in convert_sf_to_sp_type
. So technically the most "rigorous" way of writing this is checking the column type in convert_result_meta_to_attribute
and define convert_sf_to_sp_type
separately.
cde07a0
to
b42d9a9
Compare
Sorry all, I messed up a rebase and had to create a new test pr: #1095 |
@@ -90,7 +93,10 @@ def convert_result_meta_to_attribute(meta: List[ResultMetadata]) -> List[Attribu | |||
Attribute( | |||
quoted_name, | |||
convert_sf_to_sp_type( | |||
FIELD_ID_TO_NAME[type_value], precision, scale, internal_size | |||
FIELD_ID_TO_NAME[type_value], | |||
precision or 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.
What happens today if it is None
?
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.
🚢 🚢
After discussing with @sfc-gh-mkeller , we have decided to use |
This is an attempt to make some progress on #1055. Before this PR, pyright reports 625 errors, afterwards it reports 468. In particular I resolved all the type errors in
src/snowflake/snowpark/_internal/analyzer/
.