Added inspect_points datashader operation

[jlstevens]

WIP.

Here is the example code

import numpy as np
from spatialpandas.geometry import PointArray
from spatialpandas import GeoDataFrame
from holoviews.operation.datashader import rasterize, dynspread, inspect_sample
import holoviews as hv
hv.extension('bokeh')

def gaussian_df(specs=(1.5,0,1.0),num=10000):
    np.random.seed(1)
    xs, ys = np.random.normal(specs[0],specs[2],num), np.random.normal(specs[1],specs[2],num)
    return xs,ys

xs, ys = gaussian_df()
sdf = GeoDataFrame({'geometry':PointArray((xs, ys)),'id':list(range(len(xs)))})
rasterized = dynspread(rasterize(hv.Points(sdf)))
rasterized * inspect_sample(rasterized).opts(color='red', tools=['hover'], size=6)

Getting tap behavior is as easy as adding streams=[hv.streams.Tap] to the operation:

TODO

• Make sure vdims of original data are preserved for hover information
• Find out why the initial render looks blank until a small zoom in/out occurs
• Handle normal dataframes (getting the x/y from the raster kdims), geopandas, and spatially indexed spatialpandas dataframes.
• Tests
• Documentation

[jlstevens]

I might prefer inspect_point in the end as Points is the return type and it is only suited to datashaded points right now. You can imagine fancier things in future e.g inspect_segments, inspect_paths, inspect_spikes, inspect_polygons etc etc.

[jbednar]

Looks great so far!

[jbednar]

Can you maybe make the default marker shape an o (open circle) so that the marker doesn't cover up the datashaded data point?

[jlstevens]

Operations shouldn't set those kinds of options. It is easy enough to add fill_alpha=0. e.g:

An X marker is another good choice:

Would be happy to document these as examples of course.

[jlstevens]

Here is an example of how I expect the drilldown table to work:

[jlstevens]

Added point_count parameter so this operation can live up to the name inspect_points (i.e plural). Here is an example with (a maximum of) ten points shown within an expanded mask size of 50:

[jlstevens]

I think this PR will probably need some of the fixes planned in #4792

[jlstevens]

One comment Philipp made in #4796 (now closed) is that this operation should handle RGBs as well as RGBAs. I suggest raising an exception unless we want to declare a specific color as a null value.

[jbednar]

I suggest raising an exception unless we want to declare a specific color as a null value.

Is it essential to detect a null value, or just helpful? If it's not essential, I'd just print a helpful warning and then just skip checking for nulls, unless that makes it significantly more complex.

[jlstevens]

Is it essential to detect a null value, or just helpful?

It is not essential but if you can't detect null you can't optimize when inspecting sparse data. A warning seems fine.

[jlstevens]

To make sure this isn't forgotten, I realized one bug we have: the null test should be over the mask area and not just a point test (i.e use the pixels parameter).

We can also have an optimization where we pick the first sample we find falling within a single pixel (assuming anything after that doesn't matter).

[jlstevens]

Other things I would like to attempt before merging:

• Support normal dataframes without a .cx method using normal slicing, issuing a warning recommending spatialpandas/geopandas. Maybe this warning could be disabled by a parameter if people really want to avoid those dependencies.
• Output a Polygon in the shape of the mask instead of the point. Seeing the actual search area seems really useful and would make things like drilldown histograms more useful.

Other than that (and any unaddressed items above), I am very happy with this new operation! At a later date we can consider inspect_segments, inspect_paths, inspect_polygons etc..

Edit: Glancing at the code the plain dataframe support looks like it might already be there. The idea of a warning remains though...

[jlstevens]

Added an inspect_mask operation to show the mask that can be used as follows:

I think it could be improved further: ideally inspector.mask(rasterized) could be simply inspector.mask but then the processed element would need to be linked between the two operations.

[jlstevens]

@jbednar @philippjfr Ready for review. Hoping for some feedback on the API (e.g parameter names, the mask operation, arrangement of the transformers. Once things settle, I'll add unit tests. Happy to add docs in this PR but can also open a new PR if we want the code merged sooner.

[jbednar]

This will be very useful. I find the names transformer, hits_transformer, and points_transformer confusing (and verbose), though. E.g. doesn't transformer also transform hits, just as much as hits_transformer does? Maybematch_transform, matches_to_points, matches_to_hits?

It would be nice if this operation can be passed Points as an argument instead of being specialized for points, but I can't see how that could work.

Will it be able to e.g. select an entire connected trajectory, given one point on that trajectory?

[jbednar]

Again, I'm not sure that's well defined; seems like all datapoints falling into the cursor's pixel are equivalent, and they don't need to be the closest (e.g. to the cursor pixel's center) to be appropriate; I'd just take any of them as there's no particular order that can be established on them.

[jlstevens]

I'd just take any of them as there's no particular order that can be established on them.

That is a possible optimization and not true in all cases.

If the datashading is occurring at screen resolution and bokeh's hover/tap is at the same resolution then it is true (well, the above is perfectly accurate but limited by the precision supplied by Bokeh), but it is very easy to ask rasterize or datashade to output rasters where that is not true at all (i.e bins much bigger than screen pixels by specifying custom x_sampling and y_sampling).

As currently defined, this inspector will work correctly for all datashaded output and this docstring is correct. For a lot of common datashader output where things are configured to automatically make datashaded bins match screen pixels, then your suggested change would work because Bokeh isn't giving anything past screen pixel precision (and even this might not be the case, for all I know a mouse pointer can specify an x,y position that is finer than a pixel! What happens if you stick your screen into 640x480 resolution?).

Because of all this, I think the docstring is well defined and correct even if the precision with which the cursor's position is specified is limited.

[jlstevens]

I've addressed quite a few of your suggestions, thanks! The remaining items need a bit more discussion though.

Will it be able to e.g. select an entire connected trajectory, given one point on that trajectory?

No. This operation is explicitly inspect_points and I've mentioned earlier that there could be inspect_paths, inspect_segments etc. What you suggest will be possible when we flesh out this suite of operations.

Maybe match_transform, matches_to_points, matches_to_hits?

Happy to use these though I don't find them particularly compelling either. Maybe masked_transform, masked_to_points and masked_to_hits? I don't see enough of a difference between the semantics of 'matches' and 'hits' otherwise. @philippjfr any other suggestions? What do you prefer?

[philippjfr]

Could then provide a generic inspect operation which wraps around element specific ones much like rasterize does.

[jlstevens]

Could then provide a generic inspect operation which wraps around element specific ones much like rasterize does.

I like this but how would inspect know what kind of thing was datashaded/rasterized? Or in other words, what kind of element was turned into a raster? I think this might be possible by inspecting .pipeline but not via .dataset...what would you suggest?

[jlstevens]

The dask pinning is now in #4803 and we can decide what to do there. Tests on this PR will be failing unless we reintroduce the pin, fix the underlying issue (e.g update the tests?) or rebase after that PR is merged to master.

[jlstevens]

Regarding naming, here are some generic (so that we have support inspect_polygons) semantic names we want to improve:

common_dataframe_transform: Suggestions: transform_element, match_transform
output_element_data_transform: Suggestions transform, matches_to_points, mask_to_points
output_dataframe_parameter_transform: Suggestions transform_hits, matches_to_hits, mask_to_hits

[jlstevens]

I've rebased off master which now has the dask testing fixed. Before I can write tests, it looks like dynamic=False needs fixed:

[jlstevens]

Ok, dynamic=False works if you apply it to both operations:

[jlstevens]

@jbednar @philippjfr All tests are passing and I am happy with the API. There are a few things left to do but I think we can merge now and cut a dev release:

• Unit tests for the polygon inspector.
• Documentation

[jlstevens]

Going ahead and merging.

[github-actions]

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.