-
Notifications
You must be signed in to change notification settings - Fork 61
perf: Default to interactive display for SQL in anywidget mode #2138
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
base: main
Are you sure you want to change the base?
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
|
||
widget = display.TableWidget(self.copy()) | ||
return widget._repr_html_() # Return widget's HTML representation | ||
except (AttributeError, ValueError, ImportError): |
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.
Why are we catching so many exceptions here?
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.
We are catching those exceptions to handle cases where the interactive anywidget table can't be displayed.
anywidget is an optional install, so we have to catch the ImportError in case someone doesn't have it. It also only works in environments like a Jupyter notebook, so if you run the code in a regular script, it can throw an AttributeError or ValueError.
That try...except block is just a safety net. It lets us fall back to the basic table view instead of crashing the program if the setup isn't perfect for the fancier, interactive one.
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 b/448126500 I suggest investigating the errors that happen when visualizing STRUCT columns, but this PR doesn't test such cases.
dfa9c03
to
43a938c
Compare
Previously, SQL queries in anywidget mode would fall back to deferred execution, showing a dry run instead of an interactive table. This change modifies the display logic to directly use the anywidget interactive display for SQL queries, providing a more consistent and responsive user experience. A test case has been added to verify this behavior.
Adds a test case to verify that a DataFrame with a STRUCT column is correctly displayed in anywidget mode. This test confirms that displaying a STRUCT column does not raise an exception that would trigger the fallback to the deferred representation. It mocks `IPython.display.display` to capture the `TableWidget` instance and asserts that the rendered HTML contains the expected string representation of the STRUCT data.
43a938c
to
4a33ccf
Compare
Previously, SQL queries in anywidget mode would fall back to deferred execution, showing a dry run instead of an interactive table.
This change modifies the display logic to directly use the anywidget interactive display for SQL queries, providing a more consistent and responsive user experience. A test case has been added to verify this behavior.
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 #<448126500> 🦕