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

Fix key error issue of action manager #5539

Merged
merged 3 commits into from Feb 9, 2023
Merged

Conversation

melonora
Copy link
Contributor

@melonora melonora commented Feb 7, 2023

Description

Fixes the issue described in #5535. Due to the action manager bindings being incorrect, key errors were thrown when using the functionality of the labels layer.

Type of change

  • Bug-fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

References

#5535

How has this been tested?

Passed the tests in napari.layers and manually tested

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • If I included new strings, I have used trans. to make them localizable.
    For more information see our translations guide.

@github-actions github-actions bot added the qt Relates to qt label Feb 7, 2023
@melonora
Copy link
Contributor Author

melonora commented Feb 7, 2023

Errors fail due to known memory leak issue not caused by these code changes.

@codecov
Copy link

codecov bot commented Feb 7, 2023

Codecov Report

Merging #5539 (18f5f8d) into main (ef76a46) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #5539   +/-   ##
=======================================
  Coverage   89.36%   89.37%           
=======================================
  Files         608      608           
  Lines       51097    51097           
=======================================
+ Hits        45664    45669    +5     
+ Misses       5433     5428    -5     
Impacted Files Coverage Δ
napari/_qt/layer_controls/qt_labels_controls.py 91.69% <ø> (ø)
napari/layers/labels/_labels_key_bindings.py 100.00% <100.00%> (ø)
napari/_qt/dialogs/qt_package_installer.py 81.81% <0.00%> (+0.39%) ⬆️
napari/utils/theme.py 94.04% <0.00%> (+0.59%) ⬆️
napari/utils/info.py 85.10% <0.00%> (+1.06%) ⬆️
napari/_qt/__init__.py 56.66% <0.00%> (+6.66%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

@Czaki
Copy link
Collaborator

Czaki commented Feb 7, 2023

starting 24h clock

@goanpeca
Copy link
Contributor

goanpeca commented Feb 7, 2023

Thanks for working on this @melonora.


@Czaki do you have some ideas how we could tests this to catch any typos in the future ?

@Czaki
Copy link
Collaborator

Czaki commented Feb 7, 2023

As I understand, we plan to move out of action manager to app_model.

And as I understand the app model approach allows to validation action existence on binding.

@Czaki Czaki added the ready to merge Last chance for comments! Will be merged in ~24h label Feb 8, 2023
@Czaki Czaki added the bugfix PR with bugfix label Feb 9, 2023
@Czaki Czaki added this to the 0.5.0 milestone Feb 9, 2023
@Czaki Czaki merged commit ea81231 into napari:main Feb 9, 2023
@Czaki Czaki removed the ready to merge Last chance for comments! Will be merged in ~24h label Feb 9, 2023
@melonora melonora deleted the Fix_issue_#5535 branch February 9, 2023 23:24
@Czaki Czaki linked an issue Feb 9, 2023 that may be closed by this pull request
kne42 added a commit to kne42/napari that referenced this pull request Feb 28, 2023
* main: (38 commits)
  Fix `test_worker_with_progress` by wait on worker end (napari#5548)
  Un-set unified title and tool bar on mac (Qt property) (napari#5533)
  Set PYTHONEXECUTABLE as part of macos fixes on (re)startup (napari#5531)
  Fix key error issue of action manager (napari#5539)
  Clean dangling widget in test (napari#5544)
  Use pytest-pretty for better log readability (napari#5525)
  Update vendoring tool to check on matplotlib colormap (napari#5181)
  MAINT: add time limit for CI. (napari#5495)
  Add show_debug notification (napari#5101)
  Overlays 2.0 (napari#4894)
  Clarify layer's editable property and separate interaction with visible property (napari#5413)
  ci(dependabot): bump docker/build-push-action from 3 to 4 (napari#5523)
  Fix opening file dialogs in PySide (napari#5492)
  [pre-commit.ci] pre-commit autoupdate (napari#5518)
  Replace flake8, isort and pyupgrade by ruff, enable additional usefull rules (napari#5513)
  MAINT: Don't format logs in log call (napari#5504)
  Fix conda avaliability check (napari#5496)
  Handle case when QtDims play thread is partially deleted (napari#5499)
  Bugfix: Add missing Enums and Flags required by PySide6 > 6.4 (napari#5480)
  Refactor Main Window status bar to improve information presentation (napari#5451)
  ...
@Czaki Czaki mentioned this pull request Jun 7, 2023
@Czaki Czaki modified the milestones: 0.5.0, 0.4.18 Jun 16, 2023
@Czaki Czaki added the triaged-0.4.18 To mark PR that is triaged in 0.4.18 release process label Jun 16, 2023
Czaki pushed a commit that referenced this pull request Jun 18, 2023
Czaki pushed a commit that referenced this pull request Jun 19, 2023
Czaki pushed a commit that referenced this pull request Jun 21, 2023
Czaki pushed a commit that referenced this pull request Jun 21, 2023
Czaki pushed a commit that referenced this pull request Jun 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix PR with bugfix qt Relates to qt triaged-0.4.18 To mark PR that is triaged in 0.4.18 release process
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Minor bug] Key errors when using functionality of label layer
4 participants