Add the ability to add filters to exploration#1599
Conversation
There was a problem hiding this comment.
Pull request overview
This pull request adds UI functionality to dynamically add and remove filters for SQL query outputs in the exploration interface. It introduces a toggle menu for selecting which columns to filter, allowing users to interactively refine their data exploration.
Changes:
- Replaced Panel widgets with panel_material_ui widgets throughout the JSONSchema component for consistent UI styling
- Added filter management UI to SQLOutput with a MenuToggle control for selecting filterable columns
- Fixed a bug in SQL filter conditions where empty range_filters could cause errors
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lumen/transforms/sql.py | Added guard to prevent appending empty range_filters list |
| lumen/schema.py | Migrated widget types from Panel to panel_material_ui and updated widget type checking |
| lumen/filters/base.py | Updated widget initialization to use param.update() and deserialize values properly |
| lumen/ai/views.py | Added filter controls UI to SQLOutput with dynamic filter add/remove functionality |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| widgets = [] | ||
| for p, schema in self.schema.items(): | ||
| if self.properties and p not in self.properties: | ||
| if p == '__len__' or self.properties and p not in self.properties: |
There was a problem hiding this comment.
The condition p == '__len__' or self.properties and p not in self.properties uses implicit boolean conversion which can be confusing. Consider adding parentheses to make the precedence explicit: p == '__len__' or (self.properties and p not in self.properties)
| if p == '__len__' or self.properties and p not in self.properties: | |
| if p == '__len__' or (self.properties and p not in self.properties): |
| controls = super().render_controls(task, interface) | ||
| items = [] | ||
| for col in self.component.data.columns: | ||
| if col not in self.component.schema: |
There was a problem hiding this comment.
Consider adding a comment explaining why columns not in the schema are skipped, as this might not be immediately obvious to future maintainers. For example: # Skip columns without schema information (e.g., internal or derived columns)
| if col not in self.component.schema: | |
| if col not in self.component.schema: | |
| # Skip columns without schema information (e.g., internal or derived columns) |
| self._filter_area.append(filt.widget) | ||
| self.component.add_filter(filt) |
There was a problem hiding this comment.
If a filter already exists in self._filters (line 503-504), it gets re-appended to both self._filter_area and self.component.filters (lines 507-508). This could lead to duplicate filters in the UI and the component's filter list. Consider checking if the filter is already active before re-adding it.
| self._filter_area.append(filt.widget) | |
| self.component.add_filter(filt) | |
| # Only add the widget and filter if they are not already active | |
| if filt.widget not in self._filter_area: | |
| self._filter_area.append(filt.widget) | |
| if filt not in self.component.filters: | |
| self.component.add_filter(filt) |
|
I think this would be cool to have in 1.2.0 |
Adds UI to add and remove filters from a
SQLOutputin the UI