Added datashader dynspread operation #1125
Conversation
philippjfr
force-pushed
the
datashader_dynspread
branch
from
f609421
to
ea914c5
Compare
holoviews/operation/datashader.py
Outdated
| pixels using a specified compositing operator. This can be useful | ||
| to make sparse plots more visible. Dynamic spreading determines | ||
| how many pixels to spread based on a density heuristic. | ||
| """ |
There was a problem hiding this comment.
Maybe point to this section of the datashader manual, to pass the buck for having to explain it in any more detail? (Not that the datashader site necessarily does, just that if someone wants more explanation, that's the place it should be provided, not in holoviews.)
holoviews/operation/datashader.py
Outdated
| max_px = param.Integer(default=2, doc=""" | ||
| Maximum number of pixels to spread on all sides. | ||
| """) | ||
|
|
There was a problem hiding this comment.
Is this the HoloViews docstring style? It's up to you, but personally I much prefer to have it tighter:
max_px = param.Integer(default=2, doc="""
Maximum number of pixels to spread on all sides.""")
There was a problem hiding this comment.
You're right, but we generally indent the docstring by four spaces.
There was a problem hiding this comment.
Oops; that's just from having a proportional font in Github's editing box. Fixed; I too always use 4.
|
Looks good, thanks! |
holoviews/operation/datashader.py
Outdated
|
|
||
| threshold = param.Number(default=0.8, doc=""" | ||
| A tuning parameter in the range [0, 1]. Indicates the minimum | ||
| value for the density heuristic.""") |
There was a problem hiding this comment.
The important thing to convey here is the directionality -- does a higher value mean more spreading, or less?
There was a problem hiding this comment.
I just copied the datashader docstring so you presumably update it there as well.
There was a problem hiding this comment.
Right; the original isn't clear either. I don't have it running here in a handy way to test it, but if you do, please check which way it works, update the docstring, and then I'll merge and update the original docstring.
There was a problem hiding this comment.
How about:
threshold = param.Number(default=0.8, bounds=(0,1), doc="""
When spreading, determines how far to spread.
Spreading starts at 1 pixel, and stops when the fraction
of adjacent non-empty pixels reaches this threshold.
Higher values give more spreading, up to the max_px
allowed.""")
There was a problem hiding this comment.
I've updated the docstring in the datashader source to say the above, now.
There was a problem hiding this comment.
Great will make the fix shortly.
holoviews/operation/datashader.py
Outdated
| pixels.""") | ||
|
|
||
| max_px = param.Integer(default=2, doc=""" | ||
| Maximum number of pixels to spread on all sides.""") |
There was a problem hiding this comment.
Is there a reason this default value does not match datashader's default value of 3? I think that could be confusing.
There was a problem hiding this comment.
Agreed, I copied from one of my examples.
philippjfr
force-pushed
the
datashader_dynspread
branch
from
9d9556b
to
aee83cd
Compare
philippjfr
force-pushed
the
datashader_dynspread
branch
from
aee83cd
to
57f5f10
Compare
|
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. |
As the title says this adds a dynspread operation which implements dynamic pixel spreading on RGB types. It works on all Image, RGB and GridImage types.