Lasso linked select #4362
Lasso linked select #4362
Conversation
| from ..operation.datashader import rasterize | ||
| except ImportError: | ||
| raise ImportError("Lasso selection on gridded data requires " | ||
| "datashader to be available.") |
There was a problem hiding this comment.
Nice use of Datashader for this purpose! Can you not fall through to the spatial_select_columnar case when datashader is not available?
There was a problem hiding this comment.
Sure, can do, although for large rasters this will be slow.
There was a problem hiding this comment.
Right. I'm not sure if it makes sense to warn in such a case; on the one hand without a warning people might not realize that there's an easy way to make it go much faster, but on the other hand, if people genuinely can't install datashader or don't want to or have a small dataset, having such a warning will be annoying.
There was a problem hiding this comment.
Tbh, spatialpandas/shapely are more difficult to install and this isn't worth the extra complexity and possible user confusion.
|
Took me basically all weekend but I also used this PR to fix issue I had encountered when applying this to various hvPlot generated DynamicMaps (but this was almost certainly a more general issue). The issue was mainly related to the fact that it was recording and replaying both dynamic operations by traversing the DynamicMap graph and the pipeline which is accumulated as operations and methods are applied. This could cause all kinds of weirdness, therefore we now use the This should not cause any actual issues since the pipeline is a far better record of all the transforms that were applied to an object and also far easier to manipulate. |
philippjfr
force-pushed
the
lasso_linked_select
branch
3 times, most recently
from
0a28198
to
8282ff1
Compare
philippjfr
force-pushed
the
lasso_linked_select
branch
6 times, most recently
from
cfcb5dc
to
2bde8b8
Compare
philippjfr
force-pushed
the
lasso_linked_select
branch
from
8cc217e
to
ca26e7c
Compare
|
Merging since there are people waiting on a dev build which includes this work. |
| for tracking data providence for user supplied functions, | ||
| which do not make use of the clone method. Should be disabled | ||
| for operations where the output is not derived from the input | ||
| and instead depends on some external state.""") |
There was a problem hiding this comment.
Should be disabled for operations where the output is not derived from the input and instead depends on some external state.
Are there any common examples that could be listed here?
|
I know the PR is merged now but I had a look anyway. The functionality is very nice and I only have one comment regarding documentation of the I suppose my only other comment is that while nice, this functionality is a little unusual in that it requires a lot of inline imports depending on what exactly is being attempted. This seems justified in this case but I wouldn't like this sort of conditional feature to become the norm. |
Implements #4359