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

Bugfix: Don't double toggle visibility for linked layers #5656

Conversation

psobolewskiPhD
Copy link
Member

Co-authored-by: Grzegorz Bokota bokota+github@gmail.com

Description

This PR stores the layer visibility states prior to toggling layers and then checks against it to ensure that layers are toggled relative to that initial state. This makes sure they are not double toggled if linked layers are involved.
Also added a test for the interaction between linked layers and toggle_visibility that fails without this PR but now passes.

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

Closes #5655

How has this been tested?

  • I added a test that fails on main but passes here
  • all tests pass with my change
  • example: I check if my changes works with both PySide and PyQt backends
    as there are small differences between the two Qt bindings.

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 tests Something related to our tests label Mar 22, 2023
@codecov
Copy link

codecov bot commented Mar 22, 2023

Codecov Report

Merging #5656 (b5b1dbb) into main (fc038f3) will increase coverage by 1.51%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #5656      +/-   ##
==========================================
+ Coverage   88.32%   89.84%   +1.51%     
==========================================
  Files         611      611              
  Lines       51541    51589      +48     
==========================================
+ Hits        45524    46348     +824     
+ Misses       6017     5241     -776     
Impacted Files Coverage Δ
napari/layers/_layer_actions.py 72.83% <100.00%> (+5.30%) ⬆️
napari/layers/_tests/test_layer_actions.py 98.76% <100.00%> (+0.55%) ⬆️

... and 59 files with indirect coverage changes

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

@psobolewskiPhD psobolewskiPhD added the ready to merge Last chance for comments! Will be merged in ~24h label Mar 23, 2023
@psobolewskiPhD psobolewskiPhD merged commit 8a793b9 into napari:main Mar 30, 2023
@psobolewskiPhD psobolewskiPhD deleted the bugfix/toggle-linked-layer-visibility branch March 30, 2023 17:36
@psobolewskiPhD psobolewskiPhD added this to the 0.5.0 milestone Mar 30, 2023
@psobolewskiPhD psobolewskiPhD removed the ready to merge Last chance for comments! Will be merged in ~24h label Mar 30, 2023
@Czaki Czaki added the bugfix PR with bugfix label Mar 30, 2023
kcpevey pushed a commit to kcpevey/napari that referenced this pull request Apr 6, 2023
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description

This PR stores the layer visibility states prior to toggling layers and
then checks against it to ensure that layers are toggled relative to
that initial state. This makes sure they are not double toggled if
linked layers are involved.
Also added a test for the interaction between linked layers and
toggle_visibility that fails without this PR but now passes.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

# References
Closes napari#5655 

# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [x] I added a test that fails on main but passes here
- [x] all tests pass with my change 

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
@psobolewskiPhD psobolewskiPhD modified the milestones: 0.5.0, 0.4.18 Apr 30, 2023
@Czaki Czaki mentioned this pull request Jun 7, 2023
Czaki added a commit that referenced this pull request Jun 19, 2023
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description

This PR stores the layer visibility states prior to toggling layers and
then checks against it to ensure that layers are toggled relative to
that initial state. This makes sure they are not double toggled if
linked layers are involved.
Also added a test for the interaction between linked layers and
toggle_visibility that fails without this PR but now passes.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

# References
Closes #5655 

# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [x] I added a test that fails on main but passes here
- [x] all tests pass with my change 

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description

This PR stores the layer visibility states prior to toggling layers and
then checks against it to ensure that layers are toggled relative to
that initial state. This makes sure they are not double toggled if
linked layers are involved.
Also added a test for the interaction between linked layers and
toggle_visibility that fails without this PR but now passes.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

# References
Closes #5655 

# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [x] I added a test that fails on main but passes here
- [x] all tests pass with my change 

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description

This PR stores the layer visibility states prior to toggling layers and
then checks against it to ensure that layers are toggled relative to
that initial state. This makes sure they are not double toggled if
linked layers are involved.
Also added a test for the interaction between linked layers and
toggle_visibility that fails without this PR but now passes.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

# References
Closes #5655 

# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [x] I added a test that fails on main but passes here
- [x] all tests pass with my change 

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Czaki added a commit that referenced this pull request Jun 21, 2023
Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>

# Description

This PR stores the layer visibility states prior to toggling layers and
then checks against it to ensure that layers are toggled relative to
that initial state. This makes sure they are not double toggled if
linked layers are involved.
Also added a test for the interaction between linked layers and
toggle_visibility that fails without this PR but now passes.

## Type of change
<!-- Please delete options that are not relevant. -->
- [x] Bug-fix (non-breaking change which fixes an issue)

# References
Closes #5655 

# How has this been tested?
<!-- Please describe the tests that you ran to verify your changes. -->
- [x] I added a test that fails on main but passes here
- [x] all tests pass with my change 

## Final checklist:
- [x] My PR is the minimum possible work for the desired functionality

Co-authored-by: Grzegorz Bokota <bokota+github@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix PR with bugfix tests Something related to our tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Bug: toggle_visibility layer action is unpredictable with linked layers
3 participants