anntzer / mplcursors

Interactive data selection cursors for Matplotlib.
https://mplcursors.readthedocs.io
zlib License
113 stars 19 forks source link

Re-focus on cursor using mouse click #34

Open oreldo opened 3 years ago

oreldo commented 3 years ago

Hi,

Love mplcursors! When I add a second cursor to my canvas, I can move it around using keyboard bindings, which is great. However, I can not shift the focus back to my first cursor by clicking on it. Is there anyway to do that?

It works well for draggable annotations. I can drag any annotaion I want on the canvas, whether it is associated with the first or second cursor. I was hoping there was a way to define that if I click on an existing annotation/cursor, it shifts the focus to the cursor, and I could also shift/drag it along the plot using keyboard/mouse.

Thanks! Orel.

anntzer commented 3 years ago

That's a great idea, now implemented on master. Please give it a try.

oreldo commented 3 years ago

Thanks! However, it's not working for me... after clicking on an old annotation, I can still only shift the new cursor around.

anntzer commented 3 years ago

Please provide a full description of how to reproduce the issue.

oreldo commented 3 years ago

OK, after reviewing my code, I think understand what happened...

There's another issue where if I have more than one plot on my canvas, the last cursors marked on all plots are always active. So if I choose one cursor on plot 1 and another on plot 2, they will both be active simultaneously and I can shift them around using my keyboard.

I only want to shift one cursor at a time, so my workaround was that I programmatically change the bindings on all cursors but the last one. However, this means that I can't go back to previous cursors using your new feature.

Any ideas how to improve? Can I get a callback from an annotation which has been dragged so that I can change bindings back on that specific cursor?

Thanks again!

anntzer commented 3 years ago

I pushed another change that makes cursor.enabled also affect whether motion keys work (so now you can do cursor.enabled = False to make it unresponsive to motion keys). If you want to run your own callback when an annotation is dragged, I think you can connect a pick_event callback and see when the annotation is picked (then you can compare with sel.annotation for sel in cursor.selections to figure out which selection it corresponded to).

oreldo commented 3 years ago

Thanks, that works. It took me a while to figure out that _pickevent refers to the matplotlib package and not mplcursors. I run it through _mplconnect and it works well.

However, I don't think I can use cursor.enabled, because that also disables new cursors from registering for a particular plot. So I've kept the binding workaround for now. I'll be happy to know if you think there's a better, more elegant solution.

I have one more question - is there a way to define other binding keys, such as the DEL key to delete an active cursor?

Thank you for your help.

anntzer commented 3 years ago

However, I don't think I can use cursor.enabled, because that also disables new cursors from registering for a particular plot. So I've kept the binding workaround for now. I'll be happy to know if you think there's a better, more elegant solution.

To be honest, not much thought went into the interaction between multi-cursors and keybindings, because I didn't need that. So I don't have anything great to propose right now.

I have one more question - is there a way to define other binding keys, such as the DEL key to delete an active cursor?

Not right now (because until your proposal there wasn't even a notion of "current" cursor -- again, multi-cursors were sort of a side-feature), but I'd be open e.g. add a deselect_current entry to bindings with that effect. OTOH I think it's fair for me to leave the actual implementation to you -- care to open a PR? :)

oreldo commented 3 years ago

I'd be happy to help, although I will need an explanation of how to do it properly, since I do not have much experience with PRs or Git. :)

I think the entire issue around active cursors is potentially something to work on, but is not necessarily relevant to my bindings suggestion. Anything defined in bindings works on all active cursors, so defining a keyboard DEL key will essentially just have the same functionality as mouse button 3 for all active cursors. For now, I can use my workaround to define different bindings for the 'delete' function in order to make sure that only my "active" cursor will actually be deleted using DEL.

I think active/inactive cursors should be about modifying the status of a particular selection. Maybe the best way would be to change the default behavior so that only the last created selection is active and all others are inactive unless clicked on. I do think it's best to have only one cursor active at a time, unless defined otherwise.

How does that sound?

anntzer commented 3 years ago

For instructions on git/prs/github, consider checking matplotlib's contribution docs (https://matplotlib.org/stable/devel/contributing.html), including their gitter channel. I don't have much time right now for evaluating the details of the feature, but I think I agree that having just one active annotation at a time makes sense.

oreldo commented 3 years ago

Hi,

I've added the following features to my version of the mplcursors code, which I find very useful:

  1. Added DELETE key to bindings for deleting selections using keyboard.
  2. Added ENABLED property to cursor. When False, selections do not respond to bindings (left, right, up, down, delete).
  3. Added "add2" callback, linked to "select2" binding, for adding a different annotation (say, by using a different mouse button when adding a selection). This feature required an additional field added to a Selection ("button") containing the mouse button or callback used to create it.

I'd be happy to get your opinion.

In addition, I would really like to graphically mark the active cursor by changing the style of the annotation box. I'm trying to achieve this by using the following for the add callback:

    def curse_annotations(sel):
        sel.annotation.set(text="x: %0.3f\ny: %0.3f" %(sel.target[0], sel.target[1]))
        sel.annotation.get_bbox_patch().set(lw=1)
        for cur in self.all_cursors:
            for sell in cur.selections:
                if sel != sell:
                    sell.annotation.get_bbox_patch().set(lw=0)
            if cur.artists[0] == sel.artist:
                cur.active = True
            else:
                cur.active = False

So active annotations are with linewidth=1 and inactive (existing) annotations with linewidth=0. However, the figure does not refresh after changing the annotation for existing cursors, and they are all left with linewidth=1. When I press left/right to move the active cursor around, the figure refreshes and everything looks great. I tried to add a self.draw() operation at the end of the callback in order to refresh without pressing any buttons, but it throws an error. Any ideas?

anntzer commented 3 years ago

1) and 2) seem fine (though things may need to be clarified regarding how your "enabled" attribute differs from the already existing Cursor.enabled). Regarding 3) I think it may be easier to just have an event field on the Selection which is the MouseEvent (or KeyEvent, etc.) at the source of the event; this way you can know the button but also if there was a keyboard modifier, etc.

As for the last question, I think you need to call figure.canvas.draw()? (you need to get figure from sel of course)

oreldo commented 3 years ago

Regarding (2), I wrote ENABLED, but actually the new property name is ACTIVE, and is only relevant to bindings. If I am not mistaken, ENABLED = False also disables new selections from being generated. I think those are two different functionalities... but maybe they shouldn't overlap?

Regarding (3), I've changed the Selection.button field to Selection.event, which now includes the general event used to add the selection - MouseEvent, KeyEvent, etc...

figure.canvas.draw_idle() is working! Thanks!

anntzer commented 3 years ago

Regarding (2), I wrote ENABLED, but actually the new property name is ACTIVE, and is only relevant to bindings. If I am not mistaken, ENABLED = False also disables new selections from being generated. I think those are two different functionalities... but maybe they shouldn't overlap?

I guess so. One of the things that slightly worry me is simply the complex interaction between all the features here; some table summarizing all the possible combos may help.