-
Notifications
You must be signed in to change notification settings - Fork 92
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
Data Explorer: Implement partial state eviction when row filters become invalid for pandas, add error messages, invalid filter state, fix row filter bar state synchronization #2827
Conversation
event. Rename a bunch of stuff, consolidate code going between UI filter descriptors and backend RPC types
…ield to RowFilter
I'll add some more videos going through different workflows and write up some code examples Flow when a filtered column is deleted:
Screencast.from.2024-04-21.18-53-39.webm |
Filter survives a column permutation
Screencast.from.2024-04-21.19-44-33.webm |
This also bounces the filter error from #2759 into the filter modal. I don't want to declare that issue fully resolved because in principal |
One thing I saw that I would like to fix here is that when a filter is invalidated, and then as a result of a schema update, the invalid filter will work again, that it can be set to valid again. I'll figure that out tomorrow and fix the CI build |
Code changes LGTM! Some issues from my testing though:
Unrelated to this PR, but I noticed that when you apply a filter, there's no indication of how many rows the table has before versus after filtering. The row + column count on the bottom left only shows after filtering. |
I re-ran the two scenarios off the latest just now and it still works for me, maybe your kernel was stale? Screencast.from.2024-04-22.09-54-09.webmThanks for catching the invalid flicker, I just pushed a fix for that |
Good catch. We may need to return both the filtered and unfiltered shape in the |
I'm done making functional changes now and am just going to kick the tires a bit and make sure nothing else is obviously broken. |
Deleting a column that was sorted borks things, so I'm going to try to triage that before this gets merged. EDIT: there is more work here than expected so I'm going to punt this to a follow up PR. Sorting still works fine, just we will need to make some changes to handle state synchronization after a schema update. We also need to fix #2803 |
I was able to fix the sort key state syncing minimally |
There is some more tire kicking and probably small fixes needed here, but wanted to get this up for review.
General summary:
error_message
optional field toRowFilter
so that error messages from the backend can be shown in the dropdown modal. Maybe we will want to handle these messages differently, but this at least gives us a startRowFilter.column_index
toRowFilter.column_schema
. This makes it easier to detect schema changes that affect filters and make them shift or become invalidruff
when formatting the generated Positron commsAddresses #2584, #2644, #2326