-
Notifications
You must be signed in to change notification settings - Fork 106
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-1526422, SNOW-1524697: Remove axis labels and callable names from unsupported aggregation messages. #1915
SNOW-1526422, SNOW-1524697: Remove axis labels and callable names from unsupported aggregation messages. #1915
Conversation
…m unsupported aggregation messages. Signed-off-by: sfc-gh-mvashishtha <mahesh.vashishtha@snowflake.com>
…orted-aggregation-messages
Signed-off-by: sfc-gh-mvashishtha <mahesh.vashishtha@snowflake.com>
@@ -117,7 +117,7 @@ def test_unsupported_dataframe_binary_methods(func, func_name, caplog) -> None: | |||
"func, func_name", | |||
UNSUPPORTED_BINARY_METHODS, | |||
) | |||
@sql_count_checker(query_count=1) | |||
@sql_count_checker(query_count=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.
Before this PR, we would execute a query to add the entire other
series to the error message. After this PR, we don't include the other series in the error message.
Signed-off-by: sfc-gh-mvashishtha <mahesh.vashishtha@snowflake.com>
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.
left a couple ?s
src/snowflake/snowpark/modin/plugin/compiler/snowflake_query_compiler.py
Show resolved
Hide resolved
…orted-aggregation-messages
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.
Left some minor comments, but lgtm otherwise.
src/snowflake/snowpark/modin/plugin/_internal/aggregation_utils.py
Outdated
Show resolved
Hide resolved
Co-authored-by: Rehan Durrani <rehan.durrani@snowflake.com>
Signed-off-by: sfc-gh-mvashishtha <mahesh.vashishtha@snowflake.com>
…orted-aggregation-messages
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.
left a quick q, but LGTM
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.
LGTM
Along the way, separate out the
NotImplementedError
messages about unsupported aggregations from the messages about unsupported groupings (e.g. grouping onaxis=1
or grouping by a dataframe).Fixes SNOW-1526422, SNOW-1524697