fix: DataFilterExtension get_filter_category #884
Conversation
…alues of filter_size and category_size to 0 so DFE can use just a range or category filter
ATL2001
changed the title
|
a couple screenshots to show the selector changing the filtered points on my map: |
lonboard/layer_extension.py
Outdated
| } | ||
|
|
||
| filter_size = t.Int(None, min=1, max=4, allow_none=True).tag(sync=True) | ||
| filter_size = t.Int(None, min=0, max=4, allow_none=True).tag(sync=True) |
There was a problem hiding this comment.
The JS docs say
Set to
0to disable numeric filtering
I think instead of exposing that same behavior to Python, we should override None to mean "disabled". So when the Python side passes in None it translates to passing 0 into JS.
Similarly, we should change the default value here to 1 to match JS, now that None doesn't mean "undefined" but rather "defined and null"
lonboard/layer_extension.py
Outdated
| """ | ||
|
|
||
| category_size = t.Int(None, min=1, max=4, allow_none=True).tag(sync=True) | ||
| category_size = t.Int(None, min=0, max=4, allow_none=True).tag(sync=True) |
There was a problem hiding this comment.
Ditto for the same behavior as above, though the default here can be None
| return value.rechunk(max_chunksize=obj._rows_per_chunk) | ||
|
|
||
|
|
||
| class FilterCategoryAccessor(FixedErrorTraitType): |
There was a problem hiding this comment.
Can you add some tests for this? There are some example tests in test_traits.py and you can look at #917 for more examples.
It might be worth making test_traits into a folder and having a file specifically for test_traits/test_filter_extension.py
There was a problem hiding this comment.
ran out of time tonight, but I'll see what I can do in the next couple evenings, or maybe this weekend
src/model/extension.ts
Outdated
|
|
||
| extensionInstance(): _DataFilterExtension | null { | ||
| if (isDefined(this.filterSize)) { | ||
| if (isDefined(this.filterSize) && isDefined(this.categorySize)) { |
There was a problem hiding this comment.
Can we avoid having this extra case? Can we leave it to the user to define their filter sizes and category sizes correctly? I.e. why would this case need to be different from the case below it?
There was a problem hiding this comment.
Oh, whoops! If I change that && to an || then we won't need either of the the other else if blocks at all since in the creation of props it's checking isDefined too
| The data filter can show/hide data based on 1-4 numeric properties of each object. | ||
| - Type: `int`. This is required if using range-based filtering. |
There was a problem hiding this comment.
int or None. You can set None to turn off the numeric filtering, e.g. to use only category filtering, is that right?
|
ugh, I don't know what I've done over here (I honestly don't think I did anything, but clearly I must have). Friday night I went in to modify that docstring and start making some tests, and I started getting the I'm starting to think it may be a better use of my time to abandon this branch and start over from scratch 😞 |
2 participants
as we discussed in discussion #711 I was unable to get the DataFilterExtension to work for filtering data with categories.
I finally spent some time looking into what was going on, and the
FilterValueAccessorwas being used for the get_filter_category property, but that accessor is set up to work in conjunction with thefilter_sizeparameter of the DataFilterExtension. I created a newFilterCategoryAccessoraccessor modeled after theFilterValueAccessorthat works with thecategory_sizeparameter and does not do any casting of values to Float32, because looking at the DeckGL docs it appears that the category_filter should be able to work with string data.Using the new accessor for the get_filter_category on the data filter extension I'm now able to change
filter_categorieson the layer and the features are filtered as expected for numeric data. What I've got still isn't working for filtering with string data, but I figured this was worth throwing out there to see if anyone else had an idea how to get the filter working with those as well.I've added a notebook to the examples folder that I do not intend to actually check into the repo, which creates a DFE for a layer and links it to a widget to show that the category_filter is working for the numeric data, but not the string data. If we get the strings working I'd be happy to make a better example notebook that showcases category filters