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: Disconnect callbacks on object deletion in special functions from event_utils #5826

Merged
merged 3 commits into from
May 25, 2023

Conversation

Czaki
Copy link
Collaborator

@Czaki Czaki commented May 10, 2023

Fixes/Closes

Closes #5808

Description

Adds cleaning callback steps on object deletion and adds proper tests.

References

Type of change

  • Bug-fix (non-breaking change which fixes an issue)

How has this been tested?

  • example: added tests to cover the bug and fix
  • example: all tests pass with my change

Final checklist:

  • My PR is the minimum possible work for the desired functionality
  • I have commented my code, particularly in hard-to-understand areas

@Czaki Czaki added the bugfix PR with bugfix label May 10, 2023
@Czaki Czaki added this to the 0.4.18 milestone May 10, 2023
@github-actions github-actions bot added the tests Something related to our tests label May 10, 2023
@codecov
Copy link

codecov bot commented May 11, 2023

Codecov Report

Merging #5826 (08ef873) into main (d8dd1d2) will increase coverage by 0.02%.
The diff coverage is 100.00%.

❗ Current head 08ef873 differs from pull request most recent head 4c49c72. Consider uploading reports for the commit 4c49c72 to get more accurate results

@@            Coverage Diff             @@
##             main    #5826      +/-   ##
==========================================
+ Coverage   89.90%   89.93%   +0.02%     
==========================================
  Files         615      616       +1     
  Lines       52322    52372      +50     
==========================================
+ Hits        47041    47101      +60     
+ Misses       5281     5271      -10     
Impacted Files Coverage Δ
napari/utils/events/event.py 86.06% <ø> (ø)
napari/utils/events/_tests/test_event_utils.py 100.00% <100.00%> (ø)
napari/utils/events/event_utils.py 100.00% <100.00%> (+4.76%) ⬆️

... and 4 files with indirect coverage changes

@Czaki Czaki requested a review from tlambert03 May 11, 2023 06:58
@Czaki Czaki added the ready to merge Last chance for comments! Will be merged in ~24h label May 11, 2023
@psobolewskiPhD psobolewskiPhD changed the title Disconnect callbacks on object deletion in sepacial methods from event_utils Bugfix: Disconnect callbacks on object deletion in special methods from event_utils May 22, 2023
@psobolewskiPhD
Copy link
Member

@Czaki I cleaned up the title and OP plus the comment.

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
@Czaki Czaki changed the title Bugfix: Disconnect callbacks on object deletion in special methods from event_utils Bugfix: Disconnect callbacks on object deletion in special functions from event_utils May 22, 2023
@Czaki
Copy link
Collaborator Author

Czaki commented May 22, 2023

I have updated the title as methods is the name for functions bound to the class. I have hope that I do not break the title. Could you check?

Copy link
Member

@jni jni left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a stray comment, pretty sure it was unintentional but I've left the removal as a suggestion. Otherwise, 👍

@Czaki
Copy link
Collaborator Author

Czaki commented May 23, 2023

@jni read comment in the first function. I may copy it to all. The general problem is that the proper way to do this is to use finalize, but it causes segfaults. I have a hope that Qt6 may work for this better, so I prefer not to remove it but left commented until drop qt5/

@jni jni merged commit 8ad5af3 into napari:main May 25, 2023
26 checks passed
@jni jni deleted the bugfix/setattr_callbacks branch May 25, 2023 08:57
@Czaki Czaki removed the ready to merge Last chance for comments! Will be merged in ~24h label May 25, 2023
@Czaki Czaki mentioned this pull request Jun 7, 2023
Czaki added a commit that referenced this pull request Jun 19, 2023
…from `event_utils` (#5826)

# Fixes/Closes

Closes #5808

# Description

Adds cleaning callback steps on object deletion and adds proper tests. 

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Czaki added a commit that referenced this pull request Jun 21, 2023
…from `event_utils` (#5826)

# Fixes/Closes

Closes #5808

# Description

Adds cleaning callback steps on object deletion and adds proper tests. 

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Czaki added a commit that referenced this pull request Jun 21, 2023
…from `event_utils` (#5826)

# Fixes/Closes

Closes #5808

# Description

Adds cleaning callback steps on object deletion and adds proper tests. 

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.com>
Czaki added a commit that referenced this pull request Jun 21, 2023
…from `event_utils` (#5826)

# Fixes/Closes

Closes #5808

# Description

Adds cleaning callback steps on object deletion and adds proper tests. 

---------

Co-authored-by: Peter Sobolewski <76622105+psobolewskiPhD@users.noreply.github.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.

AttributeError when changing loop_mode preference with 3D+ image open
4 participants