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

Keybinding to set a new label doesn't work consistently #6517

Closed
melissawm opened this issue Dec 6, 2023 · 5 comments · Fixed by #6546
Closed

Keybinding to set a new label doesn't work consistently #6517

melissawm opened this issue Dec 6, 2023 · 5 comments · Fixed by #6546
Labels
bug Something isn't working UI/UX

Comments

@melissawm
Copy link
Member

🐛 Bug Report

After creating a labels layer, I observed that the m keybinding does not behave consistently.

💡 Steps to Reproduce

Create a labels layer.

  • If the layer is selected, the m keybinding will either do nothing or select another layer wiht a name that starts with the letter m;
  • If the paintbrush, fill bucket or polygon tools are selected, the m keybinding doesn't do anything;
  • If the paintbrush has been used in the canvas, the m keybinding correctly adds 1 to the label control and a new label is selected and can be used to paint.

💡 Expected Behavior

The m keybinding should increase the label number in all those situations.

🌎 Environment

napari: 0.5.0a2.dev486+g4d60a7ce
Platform: Linux-6.1.64-1-MANJARO-x86_64-with-glibc2.38
System: Manjaro Linux
Python: 3.11.6 | packaged by conda-forge | (main, Oct 3 2023, 10:40:35) [GCC 12.3.0]
Qt: 5.15.2
PyQt5: 5.15.10
NumPy: 1.26.2
SciPy: 1.11.4
Dask: 2023.11.0
VisPy: 0.14.1
magicgui: 0.8.0
superqt: 0.6.1
in-n-out: 0.1.9
app-model: 0.2.2
npe2: 0.7.3

OpenGL:

  • GL version: 4.6 (Compatibility Profile) Mesa 23.1.9-manjaro1.1
  • MAX_TEXTURE_SIZE: 16384

Screens:

  • screen 1: resolution 1920x1080, scale 1.0
  • screen 2: resolution 1920x1080, scale 1.0

Settings path:

  • /home/melissa/.config/napari/napari-dev_f5bfbd9c5d96bcb503f816d91f8db95d3b6d554f/settings.yaml
    Plugins:
  • napari: 0.5.0a2.dev486+g4d60a7ce (77 contributions)
  • napari-console: 0.0.9 (0 contributions)
  • napari-svg: 0.1.10 (2 contributions)

💡 Additional Context

No response

@melissawm melissawm added the bug Something isn't working label Dec 6, 2023
@dalthviz
Copy link
Member

Hi there, just in case, gave this a check and seems like the related logic for that shortcut is at

@register_label_action(
trans._(
"Set the currently selected label to the largest used label plus one."
),
)
def new_label(layer: Labels):
"""Set the currently selected label to the largest used label plus one."""
layer.selected_label = np.max(layer.data) + 1

Seems like it depends on the layer data to work (which I guess is the reason why it only works when you painted something). Also, maybe that is the reason why there are a couple of other shortcuts which change the current selected label regardless of layer data (- to decrease and = to increase)?

@psobolewskiPhD
Copy link
Member

psobolewskiPhD commented Dec 18, 2023

Sorry I missed this!!
Daniel is right m doesn't increase the label number, it sets it to the next unused number. So if there are no labels painted then max+1 will return 1 which is the first label (0 is background/erase).
Likewise, if you paint with 1, hit m to get 2 then hitting m again won't do anything, because it's already at max+1.
So I would say this is not a bug, but it might be worth considering a Notification in these cases just not sure how to phrase it, maybe like:
Largest label already selected or something?

@melissawm
Copy link
Member Author

Oh that makes sense, yes! A warning sounds very reasonable. Thanks!

@dalthviz
Copy link
Member

So something like this could work?:

@register_label_action(
    trans._(
        "Set the currently selected label to the largest used label plus one."
    ),
)
def new_label(layer: Labels):
    """Set the currently selected label to the largest used label plus one."""
    new_selected_label = np.max(layer.data) + 1
    if layer.selected_label == new_selected_label:
        warnings.warn(
            trans._(
                "Largest label already selected",
            )
        )
    else:
        layer.selected_label = new_selected_label

A preview:

label

Let me know if such an approach could be worthy to submit a PR :)

@psobolewskiPhD
Copy link
Member

Yeah that's exactly what I was thinking, only using show_info

def show_info(message: str):

see also:
def select_all_in_slice(layer: Points):
new_selected = set(layer._indices_view[: len(layer._view_data)])
# If all visible points are already selected, deselect the visible points
if new_selected & layer.selected_data == new_selected:
layer.selected_data = layer.selected_data - new_selected
show_info(
trans._(
"Deselected all points in this slice, use Shift-A to deselect all points on the layer. ({n_total} selected)",
n_total=len(layer.selected_data),
deferred=True,
)
)

It has a different icon in the notification, versus warning.

psobolewskiPhD added a commit that referenced this issue Feb 3, 2024
…l to largest used label plus one (#6546)

# References and relevant issues

Closes #6517 

# Description

Add showing a notification when the shortcut to set current selected
label to largest used label plus one can't change the label value since
it is already the largest used label plus one.

A preview:


![label](https://github.com/napari/napari/assets/16781833/e62a11e3-21dc-4cba-bc54-6e671e2d8aa8)

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working UI/UX
Projects
None yet
3 participants