-
Notifications
You must be signed in to change notification settings - Fork 48
test: fix snippets test of using partial ordering mode #1741
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
Conversation
samples/snippets/quickstart_test.py
Outdated
@@ -34,3 +34,6 @@ def test_quickstart( | ||
quickstart.run_quickstart(your_project_id) | ||
out, _ = capsys.readouterr() | ||
assert "average_body_mass:" in out | ||
|
||
# close session so not to affect other tests | ||
bigframes.pandas.close_session() |
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.
If the objective is to reset the partially ordered mode to default ordering, I'm afraid that would not happen here, as the value set stills lingers on, and picked up by the next session created
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.
right. reset the option and moved to run_quickstart() where the option is changed.
@@ -71,3 +71,7 @@ def run_quickstart(project_id: str) -> None: | ||
model.fit(X, y) | ||
model.score(X, y) | ||
# [END bigquery_bigframes_quickstart] | ||
|
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 better approach, thanks!. For additional sanity let's restore the original ordering mode. (The reason for this PR is because multiple tests are sharing the global session, in which case we give back what we got).
original_ordering_mode = bpd.options.bigquery.ordering_mode
# [START bigquery_bigframes_quickstart]
...
# [END bigquery_bigframes_quickstart]
bpd.close_session()
bpd.options.bigquery.ordering_mode = original_ordering_mode
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.
Lemme know what you think about #1743
Thank you for opening a Pull Request! Before submitting your PR, there are a few things you can do to make sure it goes smoothly:
Fixes #<issue_number_goes_here> 🦕