Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow direct creation of short annotations in interactive plots #9133

Open
mpcoll opened this issue Mar 17, 2021 · 9 comments
Open

Allow direct creation of short annotations in interactive plots #9133

mpcoll opened this issue Mar 17, 2021 · 9 comments
Labels
BUG sprint-2023 Issues reserved for the 2023 Intermediate Dev Training VIZ

Comments

@mpcoll
Copy link

mpcoll commented Mar 17, 2021

Describe the new feature or enhancement

At the moment, when annotating interactively on a raw.plot(), any annotation created by click and dragging disappears and is not saved on release if it is shorter than ~ 100 ms.
However, once created, annotations can be reduced to a shorter duration.

For some applications (e.g. manually marking peaks, adding events), it could be useful to be able to directly create short or single-sample annotations in one step/single-click.

@mpcoll mpcoll added the ENH label Mar 17, 2021
@drammock drammock added BUG VIZ and removed ENH labels Mar 17, 2021
@drammock
Copy link
Member

Annotations can in principle have duration of 0, so I guess there's no reason we couldn't create annotations on single click... I think the original thinking was that annotations that are too short are hard to right-click on if you want to remove them later. @larsoner any opinion here?

@cbrnr
Copy link
Contributor

cbrnr commented Mar 18, 2021

Let's not create short annotations with a single click, but rather stick with click and drag. That way, accidental clicks into the plot won't create annotations, which I'm sure would be very annoying. Clicking and dragging could be modified to support shorter durations though. I guess at the very least we could reduce the current threshold - is it a constant 100ms or does it depend on the currently visible data segment and/or sampling frequency?

@mpcoll
Copy link
Author

mpcoll commented Mar 19, 2021

Thanks!
I did a few tests and setting minspan to None in the selector (_figure.py line 1552) allows the creation of a 0 duration annotation with a single click. It can't be deleted with a right click but if draggable edge is turned on, it can be extended and then deleted easily.

To be able to directly delete with a right-click and no draggable edge, minspan=(1/sfreq)*2 works but needs a bit of click spamming in the area before catching it and I have not tested this extensively.

Setting minspan to none works for my purpose but I guess the optimal setting here will depend on the most common use case for this tool. Maybe the single click option could be turned on only if self.mne.draggable_annotations.

@drammock
Copy link
Member

Let's not create short annotations with a single click, but rather stick with click and drag.

Personally I see "single click to create zero-duration annotation" as a valid use case.

setting minspan to None in the selector (_figure.py line 1552) allows the creation of a 0 duration annotation with a single click. It can't be deleted with a right click but if draggable edge is turned on, it can be extended and then deleted easily.

That seems like a good enough user experience to me. I think it would require having draggable edges turned on by default; users can turn it off and be warned in the docstring and UI that zero-duration annotating will be disabled while it is off.

@agramfort @larsoner any opinions here?

@agramfort
Copy link
Member

agramfort commented Mar 19, 2021 via email

@cbrnr
Copy link
Contributor

cbrnr commented Mar 19, 2021

Can we try to make right-click delete work? Then this would be independent of the draggable edges setting.

@drammock
Copy link
Member

We could in principle increase the "hit" tolerance on the zero-duration annotations, so that right-clicking within 3-4 pixels of it would count as a hit. Seems worth trying out. If the zero-duration annotation overlapped another annotation of a different type, the behavior is hard to predict... but nowadays we can dynamically show/hide different annotation types through the UI, so it is possible to avoid such situations.

@mpcoll
Copy link
Author

mpcoll commented Mar 19, 2021

Can we try to make right-click delete work? Then this would be independent of the draggable edges setting.

Another potential solution might be to replace the right-click delete with a right-button SpanSelector and clear all annotations within the right-button selector. It also has the added benefit of being able to delete multiple annotations at once.

I tried it and it seems to work well enough but maybe it comes with some other downside I can't see

Added this after _select_annotation_span ~ line 1332

    def _clear_annotations_span(self, vmin, vmax):
        """Handle annotation span clearer."""
        onset = _sync_onset(self.mne.inst, vmin, True) - self.mne.first_time
        offset = onset + (vmax - vmin)
        to_delete = []
        for idx, an in enumerate(self.mne.inst.annotations):
            if onset <= an['onset'] <= offset or onset <= (an['onset'] + an['duration']) <= offset:
                # only remove visible annotation spans
                if self.mne.visible_annotations[an['description']]:
                    to_delete.append(idx)
        self.mne.inst.annotations.delete(to_delete)
        self._remove_annotation_hover_line()
        self._draw_annotations()
        self.canvas.draw_idle()

        self._redraw(update_data=False, annotations=True)

Added the selector in the _create_annotation_fig function:

  selector = SpanSelector(self.mne.ax_main, self._select_annotation_span,
                          'horizontal', minspan=None, useblit=False,
                          button=1,
                          rectprops=dict(alpha=0.5, facecolor=col))
  # Right click selector to clear all annotations within
  cl_selector = SpanSelector(self.mne.ax_main, self._clear_annotations_span,
                          'horizontal', minspan=None, useblit=False,
                          button=3, span_stays=False,
                          rectprops=dict(alpha=0.5, facecolor='gray'))
  self.mne.ax_main.cl_selector = cl_selector

I also removed the if annotating in the right-click button press in _buttonpress:

def _buttonpress(self, event):
   """Handle mouse clicks."""
   butterfly = self.mne.butterfly
   annotating = self.mne.fig_annotation is not None
   ax_main = self.mne.ax_main
   inst = self.mne.inst
   # ignore middle clicks, scroll wheel events, and clicks outside axes
   if event.button not in (1, 3) or event.inaxes is None:
       return
   elif event.button == 1:  # left-click (primary)
       # click in main axes
       if (event.inaxes == ax_main and not annotating):
           if self.mne.instance_type == 'epochs' or not butterfly:
               for line in self.mne.traces + self.mne.epoch_traces:
                   if line.contains(event)[0]:
                       if self.mne.instance_type == 'epochs':
                           self._toggle_bad_epoch(event)
                       else:
                           idx = self.mne.traces.index(line)
                           self._toggle_bad_channel(idx)
                       return
           self._show_vline(event.xdata)  # butterfly / not on data trace
           return
       # click in vertical scrollbar
       elif event.inaxes == self.mne.ax_vscroll:
           if self.mne.fig_selection is not None:
               self._change_selection_vscroll(event)
           elif self._check_update_vscroll_clicked(event):
               self._redraw()
       # click in horizontal scrollbar
       elif event.inaxes == self.mne.ax_hscroll:
           if self._check_update_hscroll_clicked(event):
               self._redraw(annotations=True)
       # click on proj button
       elif event.inaxes == self.mne.ax_proj:
           self._toggle_proj_fig(event)
       # click on help button
       elif event.inaxes == self.mne.ax_help:
           self._toggle_help_fig(event)
   else:  # right-click (secondary)
       #
       if event.inaxes == ax_main:  # hide green line
           self._blit_vline(False)

@cbrnr
Copy link
Contributor

cbrnr commented Mar 25, 2021

Let's stick to the existing UX and just increase the click toleration to 3 pixels around it, I feel that right-click and dragging is a very exotic interaction.

@drammock drammock added the sprint-2023 Issues reserved for the 2023 Intermediate Dev Training label Jul 5, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BUG sprint-2023 Issues reserved for the 2023 Intermediate Dev Training VIZ
Development

No branches or pull requests

4 participants