Implemented the Lasso select in Scatter.

[tomr-stargazer]

Hi all, this is a demonstration of how I think Lasso selections will work -- please check it out and try running it. The main hangup is that when you select a structure via other methods, astrodendro assumes you want to select all of that structure's children as well, which is not a paradigm that works with lasso select and we'll probably have to retool the code in a few places to be more general about this. Also, if you try to select more than one point at a time you'll hit a NotImplementedError.

Some scattered thoughts I wrote in my commit message are below.

The main obstacles to tackle: rethinking how SelectionHub.select() should work, now that we might use the Lasso to select multiple unrelated structures. The previous selection methods always assumed you wanted a single structure plus all its children, but now we might want to split these into two use cases:

1. You pick an object and want all its children
2. You pick a handful of unrelated objects and you don't want their children

in which case it might make sense to reimplement (1.) as a special case of (2.) which activates when you pass the select() function a single structure (and then it goes and populates the selection with all the children before passing it along to the plots) or something like this. You see what I mean?

This kind of thing would necessitate changing some of the logic in, e.g. BasicDendrogramViewer._update_lines() which calls DendrogramPlotter.get_lines() which has the code structures = structure.descendants + [structure] implying that when you plot some lines you always want descendants. I'm not super sure how hard I want to fix that, but if we wanted to have a perfect logical correlation with the scatter plots you circle and the specific branches that get highlighted, we would need to circumvent this "get all descendants" logic.

[tomr-stargazer]

@ChrisBeaumont @astrofrog I worked in some of the functionality from http://matplotlib.org/examples/event_handling/lasso_demo.html thanks to Chris's tip yesterday -- could you take a look and and let me know if you have any thoughts?

[tomr-stargazer]

@ChrisBeaumont This code works on my machine, and I believe I've implemented everything required for Lasso selections to work seamlessly. However, the last Travis build failed and I don't understand the error message. When you can, check this code out and let me know what you think -- and try it out!

If you have a little extra time, I also encourage you to try using the "integrated viewer" I built: https://github.com/tomr-stargazer/astrodendro_analysis/blob/master/integrated_viewer.py (it hooks into a Hub the same way a Scatter viewer does - example shown here: https://github.com/tomr-stargazer/astrodendro_analysis/blob/master/demo.py#L159)

[ChrisBeaumont]

I'll take a look into the failures (it's complaining about not expecting an attribute named structure somewhere, and I wonder if this is related to some of the structure->structures refactoring).

I'm happy with pulling this in, but it still needs some more documentation (both docstrings on the code users are likely to call directly, and probably some extra info in the sphinx documentation). @cfaesi is a good test particle here -- he should be able to gather what he needs from the documentation to build an interactive plot window with all the new bells and whistles.

@astrofrog are you ok with merging in more custom viewer functionality like this, or do you think this should be kept out of the main package?

[tomr-stargazer]

Ah - it looks like it broke because I changed the signature of plot.get_lines, but didn't update plot.plot_tree accordingly. I'll fix that and see if that's enough to get the tests passing again. (Also, from now on I will make sure to run py.test locally before making pull requests.)

[tomr-stargazer]

@ChrisBeaumont Here's some documentation, including a docstring (with an example) in scatter.Scatter and a new paragraph plus two screenshots in plotting.rst. Will this do?

[ChrisBeaumont]

Looks good -- I made some small style suggestions.

I tried this out, and I trigger errors if I select a region that doesn't contain any points. Can you fix that corner case?

The documentation looks good to me, but I'd like to hear if @cfaesi or @astrofrog agree. Once they are happy and the empty selection bug is fixed, I'm +1 to merge

[tomr-stargazer]

@ChrisBeaumont I believe I addressed all of your points and fixed the bug you called out. There is one final bug I'm having trouble fixing: when you click on the scatter plot and then release immediately, it seems to lock up the lasso and not allow you to draw another lasso ever again. I suspect it has to do with how the widget gets locked and unlocked, but that's about as far as I've gotten. It's the same bug that this guy reported on the matplotlib-users mailing list in 2008 (which was never responded to) : http://matplotlib.1069221.n5.nabble.com/How-to-fix-Lasso-example-bug-tc13395.html#none

[tomr-stargazer]

Update: I simply removed all lines of code surrounding the Lasso that made use of locks. Now it works great. I had inherited the lock-based code from the lasso example that I was sent a while ago, but it doesn't seem to be necessary in this code.

[ChrisBeaumont]

Everything seems to work fine without widget locking, so I'm fine with removing it

[tomr-stargazer]

I just added some more documentation (plus a couple convenience functions for making logscaled scatter plots) based on having @cfaesi give this functionality a test-drive. It's looking even better now.

[tomr-stargazer]

It's been a long time since we've brought this up, but I've had @cfaesi give me feedback on the usability of code, and I've demo'd the lasso-selections at a scientific conference, so I was curious if this is merge-able? @ChrisBeaumont @astrofrog

[ChrisBeaumont]

Looks good -- merging now