Closed jlstevens closed 1 year ago
It isn't actually the uint32
that is the problem, it is the ds.mean
. For antialiased lines there is specific handling of some reductions (count
, any
, min
, max
, sum
) but evidently not mean
.
Thanks for the clarification!
Do you think mean
could be supported with antialiasing? Would that be feasible to support and does it make semantic sense?
mean
shouldn't be a problem to support. It makes as much sense as min
and max
.
I'd say that an additional issue is that we should never be hitting a Numba error when the underlying issue is a reduction that's not been implemented for a certain case (e.g. antialiasing). I think Datashader should be catching such issues and explicitly returning a NotImplementedError, so that users can distinguish between "not implemented" and "broken".
It turns out that it is not trivial to add support for compound reductions that contain two or more underlying reductions, such as mean
that contains a sum
and count
. I have submitted a PR (#1138) that raises a NotImplementedError
for such reductions, as a short-term improvement. In the meantime I will investigate making changes to the numba dispatch mechanism used for antialiased lines to support these reductions.
There is a design issue about use of the self_intersect
kwarg if we want to support compound reductions on antialiased lines. Consider
agg = ds.summary(sum=ds.sum(self_intersect=True), count=ds.count(self_intersect=False))
canvas.lines(..., agg=agg, line_width=1)
Both reductions are calculated in a single pass of the antialiased line code, so only a single value of self_intersect
is acceptable for the pass. Note that self_intersect=True
is the default, so self_intersect=False
expresses a definite desire from the user to turn off self-intersection.
I see two possible solutions:
self_intersect=False
use this for the single pass.Reduction
classes to Canvas.lines
alongside the line_width
kwarg. This more naturally expresses what is occurring as self_intersect
is a property of the line algorithm rather than an individual reduction, although it only has relevance for a subset of reductions. But this is an API change.1 sounds fine to me, if documented.
To reproduce, note that the following code works with
line_width=0
:But fails with a numba error with a non-zero
line_width
:This issue crops up when using datashader from HoloViews when trying to datashade boolean data: HoloViews casts to
uint32
as datashader would just complain about the data being non-numeric otherwise.However, I do think there is an issue here in datasahder: either datashader should explicitly complain that it doesn't support
uint32
or otherwise there shouldn't be cases where errors occur only when antialiasing is enabled.