pyapp-kit / superqt

Missing widgets and components for Qt-python
https://pyapp-kit.github.io/superqt/
BSD 3-Clause "New" or "Revised" License
210 stars 37 forks source link

fix: fix issues with SliderLabel strong reference #248

Open tlambert03 opened 5 months ago

tlambert03 commented 5 months ago

I found a case where SliderLabel prevented cleanup by holding onto a bound method. This cleans that up

codecov[bot] commented 5 months ago

Codecov Report

Attention: Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.

Project coverage is 87.04%. Comparing base (952ac33) to head (4c57cfd).

Files with missing lines Patch % Lines
src/superqt/sliders/_labeled.py 60.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #248 +/- ## ========================================== - Coverage 87.09% 87.04% -0.05% ========================================== Files 46 46 Lines 3433 3436 +3 ========================================== + Hits 2990 2991 +1 - Misses 443 445 +2 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

tlambert03 commented 1 month ago

this is a real strange mess. The goal is to remove the partial and lambda usage in _labeled... but for some reason, merely commenting out the line:

        if connect is not None:
            self.editingFinished.connect(lambda: connect(self.value()))

in SliderLabel causes a segfault on pyside6.

not urgent, so going back in the pile for now

Czaki commented 1 month ago

@tlambert03 could you point line in code?

tlambert03 commented 1 month ago

Sure, basically the goal of this PR is to:

(apologies for the crappy code...)

Czaki commented 1 month ago

Ah. You do not use tox/nox that makes it harder to reproduce locally.

Could you try to check if store hard reference to connect when commenting out this part of code?
Because commenting out lambda connection means also lost the reference.

tlambert03 commented 1 month ago

prefer not to use nox/tox honestly.
however, i don't think it should be hard to reproduce. i just made a new environment, pip installed editable with tests, and installed the pyside6 extra (==6.7) and was able to reproduce it. i'm on mac fwiw.

don't have time to check the references now

Czaki commented 1 month ago

I'm going sleep. If you do not do this, I will try tomorrow.