Skip to content
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

Enable filtering on async tables #26

Open
jekelsc opened this issue Jun 18, 2019 · 5 comments
Open

Enable filtering on async tables #26

jekelsc opened this issue Jun 18, 2019 · 5 comments

Comments

@jekelsc
Copy link

jekelsc commented Jun 18, 2019

I want to start working on a PR to support this. Adding the issue so I can ask questions.

Believe I understand how to do this from the ag-grid side. Can you comment on any current ideas you have for filtering the data source? Still trying to understand the Julia data landscape completely. Were you thinking of using Query.jl? Or is filtering something I should build directly on Tables.jl abstractions?

@pfitzseb
Copy link
Member

I didn't actually think much on that yet :)
Just asked on Slack and it seems like Query.jl should work fine if you work with something like

table = Tables.datavaluerows(table)

Seems like it should be fairly easy to implement filtering with that, but I'm not sure if sorting is equally easy (because of performance).

If there's anything I can help with, feel free to ask!

@davidanthoff
Copy link
Contributor

You could probably even just use Base.filter here and not take a dependency on Query.jl?

jekelsc added a commit to jekelsc/TableView.jl that referenced this issue Jun 21, 2019
Enables default ag-grid style filtering on async tables by creating a filter function from filterModel data.
@jekelsc
Copy link
Author

jekelsc commented Jun 21, 2019

Added a pull request for consideration.

The PR as coded is pretty clear since it translates filterModel into Julia code that is parsed and evaluated. MetaProgramming suggestions are welcome if there is good reason to do this differently. Had to use invokelatest in order to get this to work.

The default text filter behavior is a bit annoying in that it lowercases the filter string, so ended up using regular expression matching with ignore case.

For async tables enabled "Clear filter" and "Apply filter" so that the user controls when the filtering starts.

The Base.Iterators.filter on rows should work, but have only tested with a DataFrame so far. If there are cases where we need to call Tables.datavaluerows please advise.

Also looking for hints on what to do about rowCount. The current behavior of the scrollbar seems tied to the size of the full set of rows without filtering. As filtering can only make the number of rows smaller, perhaps it is okay that one can scroll past the end of the filtered results.

Finally, the ag-grid has an issue where for the 2nd condition in a filter the user can apply the filter for "in range" even if some range fields are blank. Instead of complicating the generated code expressions with checks for === nothing, it simply throws exceptions for now.

Not sure if any automated tests are possible easily.

@jekelsc
Copy link
Author

jekelsc commented Jun 21, 2019

Also, in case anyone is wondering, used getproperty on a constructed Symbol in case the column name has spaces in it.

jekelsc added a commit to jekelsc/TableView.jl that referenced this issue Jun 21, 2019
Minor adjustment to return the invokelatest wrapped filter on the first use. The world will be ready for the actual function on subsequent uses.
jekelsc added a commit to jekelsc/TableView.jl that referenced this issue Jun 22, 2019
Added code to sanitize column access and partially specified ranges. Optimize dates via preparsing.
@jekelsc
Copy link
Author

jekelsc commented Jun 22, 2019

Code now handles partially specified ranges by ignoring them.

jekelsc added a commit to jekelsc/TableView.jl that referenced this issue Jun 23, 2019
Handle column names and search text with quote characters more defensively.
jekelsc added a commit to jekelsc/TableView.jl that referenced this issue Jun 24, 2019
Updated to generate expressions directly rather than parsing a string.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants