-
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-1491306 Phase 0 AST for DataFrame first, sample, and random_split #1813
SNOW-1491306 Phase 0 AST for DataFrame first, sample, and random_split #1813
Conversation
@@ -0,0 +1,17 @@ | |||
## TEST CASE |
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 test will not pass right now since the Local History code has not implemented any "random" functionality yet. A NotImplementedError is raised.
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.
Is this still true? One way or another, we should make sure that all (not disabled) checked in tests pass.
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 have run into this in a number of recent APIs as well. I've been able to get around this by checking session._conn._suppress_not_implemented_error, which is a new connection property I've added. In many places, I just return None if suppression is enabled.
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.
@sfc-gh-azwiegincew I'm still unable to get this test to pass. Now, the NotImplementedError is not raised but instead a KeyError is raised:
def handle_function_expression(
exp: FunctionExpression,
input_data: Union[TableEmulator, ColumnEmulator],
analyzer: "MockAnalyzer",
expr_to_alias: Dict[str, str],
current_row=None,
):
. . .
try:
> result = _MOCK_FUNCTION_IMPLEMENTATION_MAP[func_name](*to_pass_args)
E KeyError: 'random'
../../src/snowflake/snowpark/mock/_plan.py:454: KeyError
What should I do to get around this? Should I try to catch
the error and check if it's a KeyError with this message?
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.
In random_split()
, right after creating the AST, say something along the lines of if self._conn._suppress_not_implemented_error: return None
…t-sample-random-split
@@ -0,0 +1,17 @@ | |||
## TEST CASE |
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.
Is this still true? One way or another, we should make sure that all (not disabled) checked in tests pass.
Ping. |
@@ -0,0 +1,17 @@ | |||
## TEST CASE |
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.
In random_split()
, right after creating the AST, say something along the lines of if self._conn._suppress_not_implemented_error: return None
Which Jira issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes SNOW-1491306
Please describe how your code solves the related issue.
This PR adds API coverage for DataFrame first, sample, and random_split. I added tests for the same.