Repository navigation
Update Selection Widgets #3486
Description
I took a stab at updating the selection widgets. Namely, improving the RectangleSelector and LassoSelector, and adding new EllipseSelector, PaintSelector and LineSelector widgets. These are based on a proof of concept I made called image inspector, which in turn is based on the scikit-image canvastools.
I created a gist that demonstrates all the widgets.
Improvements to RectangleSelector:
- Visible handles for manipulating the shape after it has been drawn
- Keyboard modifiers:
- Alt moves the existing shape (as does dragging the center handle)
- Shift makes the shape square (or circular)
- Ctrl makes the initial point the center of the shape (helpful for ellipses)
- Ctrl and shift can be combined
- When
drawmode == 'line', actually draw the outline instead of a single line
The API is the same, but the default drawmode has been changed to 'patch', while allowing 'box' to alias to 'patch'. Also, the shape is no longer invisible after been drawn (so we can interact with it).
Improvements to LassoSelector:
- Unified API with
RectangeSelector - Keyboard modifier:
- Shift enables
polygonmode where it draws straight lines
- Shift enables
Note that LassoSelector had two methods that were renamed: onpress -> press and onrelease -> release. The old methods could be kept, and call the new methods while giving a deprecation warning.
Here is a screenshot of the Ellipse tool:
How would you like me to proceed? Throw them all into one PR?
Activity
blink1073 commentedon Sep 8, 2014
@tonysyu, @tacaswell, what are your thoughts?
tacaswell commentedon Sep 8, 2014
Can you spilt them into as many PRs as make sense? My concern is that large PRs are hard to review (which makes it easier for bugs to slip in) and provide a larger cross section to collect conflicts (leading to more re-basing).
Again, I apologize, I have not had the bandwidth to look in to this in any detail.
fariza commentedon Sep 9, 2014
@blink1073 just as a side note, this could be related #3475
WeatherGod commentedon Sep 9, 2014
On Tue, Sep 9, 2014 at 10:42 AM, Federico Ariza notifications@github.com
wrote:
fariza commentedon Sep 10, 2014
@blink1073 be aware of #3376
blink1073 commentedon Sep 11, 2014
It would be nice to merge #3376 first so I can build on that.
tonysyu commentedon Sep 17, 2014
I think what you did with image inspector is great, and a more unified API for selection widgets would be welcome. I agree with @tacaswell that small PRs would be much easier to review. Probably just the base
SelectionWidgetand one of the concrete classes in the first PR.petehuang commentedon Jan 7, 2017
Seems like there was great work done here, thanks and props @blink1073!
What is the status of this particular issue? I'm assuming there weren't any other PRs to submit, in which case we should mark this closed. Please do comment if otherwise.
blink1073 commentedon Jan 7, 2017
Thanks, @petehuang! There is an open PR for the paint selector (#7411), to be followed by a PR for the line selector. I decided not to tackle the lasso selector integration after all. I'd say this particular PR can be closed.