dendrograms / astrodendro

Generate a dendrogram from a dataset
https://dendrograms.readthedocs.io/
Other
37 stars 38 forks source link

Multiple selections #100

Closed tomr-stargazer closed 10 years ago

tomr-stargazer commented 10 years ago

I implemented the ability to select multiple dendrogram structures using the three mouse buttons (on my mac, they correspond to "click", "option-click" and "control-click" for (1, 2, 3) respectively). Some of the internal variables are now dictionaries corresponding to the mouse Event identifiers.

I am pretty sure this works overall. Is this worth merging into master?

ChrisBeaumont commented 10 years ago

Hi Tom,

This looks like a nice addition! I have some small style comments, which I'll make inline. Also, I think it's worth rebasing to collapse this PR into a single commit. If you haven't done that before, I can show you how tomorrow

ChrisBeaumont commented 10 years ago

Also, we should probably update the "Selected Structure" plot title, to display the Selected structure IDs for all 3 selections

astrofrog commented 10 years ago

@tomr-stargazer - just about to test this. Before we merge this, in addition to @ChrisBeaumont's comments, it would be good to rebase this to get rid of the merge commit. No need to squash all the commits, but if you rebase it with default parameters, it will get rid of the merge commit.

astrofrog commented 10 years ago

This works for me. I am wondering whether we should be using the different mouse buttons, or whether instead we should use e.g. numerical key modifiers, e.g. 1-click, 2-click, 3-click. This would give more options and also would leave the different mouse buttons free for other functionality, such as modifying contrast/brightness, etc?

astrofrog commented 10 years ago

I have one small request, if easy to implement. At the moment, if you select three structures, each one containing another, then there is no guarantee that the smallest one will appear on 'top' in the tree view:

screen shot 2014-03-13 at 2 27 25 pm

I'm not sure how easy it would be to deal with this cleanly, but the zorder of the colors on the right could be related to the brightness of the faintest pixel in a structure, to get around this issue.

ChrisBeaumont commented 10 years ago

Thanks, @astrofrog . I had similar feelings about zorder and numbers in some of the "comments about an outdated diff". Tom Rice said numerical key modifiers are harder to connect to pick events, so that might not be feasible at the moment.

astrofrog commented 10 years ago

Ok, that sounds good. In that case it makes sense to leave it as-is (though the zorder change should still be do-able, right?). Maybe it would be worth adding a sentence to the docs describing the functionality?

ChrisBeaumont commented 10 years ago

+1 on docs and zorder. My suggestion was to always put the most recently-selected structure in front. A smarter approach is to put the largest structures in back

tomr-stargazer commented 10 years ago

I'll take a crack at implementing these shortly.

As for the numerical key modifiers, my first approach (which didn't work with pick_events) was to have key presses replace mouse clicks when selecting map/dendrogram regions.

I could look into another implementation in which the keypresses 1-8 change which selection color is "active" but the mouse clicks are still used to actually select the regions; if this is similar to what you're both suggesting I think I can figure out a way to do it.

Thanks much for the great feedback!

Tom

On Thu, Mar 13, 2014 at 10:09 AM, Chris Beaumont notifications@github.comwrote:

+1 on docs and zorder. My suggestion was to always put the most recently-selected structure in front. A smarter approach is to put the largest structures in back

Reply to this email directly or view it on GitHubhttps://github.com/dendrograms/astrodendro/pull/100#issuecomment-37536573 .

tomr-stargazer commented 10 years ago

I got the zorder terms to work with a simple line of code:

self.selected_lines[selection_id].set_zorder(structure.height)

image

tomr-stargazer commented 10 years ago

further, applying the zorder to everything (contours, scatter plot) seems to work even better. image

image

tomr-stargazer commented 10 years ago

And, finally, display of multiple selected structures (here I have the text color matching) image image

ChrisBeaumont commented 10 years ago

@tomr-stargazer -- do I understand correctly that #104 supersedes this PR, which can be closed?

tomr-stargazer commented 10 years ago

Yes to both. On Mar 13, 2014 7:09 PM, "Chris Beaumont" notifications@github.com wrote:

@tomr-stargazer https://github.com/tomr-stargazer -- do I understand correctly that #104 https://github.com/dendrograms/astrodendro/pull/104supersedes this PR, which can be closed?

Reply to this email directly or view it on GitHubhttps://github.com/dendrograms/astrodendro/pull/100#issuecomment-37598259 .