Skip to content

fix: fix issues with SliderLabel strong reference#248

Merged
tlambert03 merged 7 commits intopyapp-kit:mainfrom
tlambert03:fixes
Mar 4, 2026
Merged

fix: fix issues with SliderLabel strong reference#248
tlambert03 merged 7 commits intopyapp-kit:mainfrom
tlambert03:fixes

Conversation

@tlambert03
Copy link
Copy Markdown
Member

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

@tlambert03 tlambert03 changed the title fix: fix issues with SliderLabel fix: fix issues with SliderLabel strong reference Jun 3, 2024
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 3, 2024

Codecov Report

❌ Patch coverage is 92.85714% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 85.24%. Comparing base (58b8cec) to head (2de69fc).
⚠️ Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
src/superqt/sliders/_labeled.py 92.85% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #248      +/-   ##
==========================================
- Coverage   87.04%   85.24%   -1.80%     
==========================================
  Files          49       49              
  Lines        3866     3871       +5     
==========================================
- Hits         3365     3300      -65     
- Misses        501      571      +70     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@tlambert03
Copy link
Copy Markdown
Member Author

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
Copy link
Copy Markdown
Contributor

Czaki commented Oct 12, 2024

@tlambert03 could you point line in code?

@tlambert03
Copy link
Copy Markdown
Member Author

tlambert03 commented Oct 12, 2024

Sure, basically the goal of this PR is to:

  • remove use of partial here
  • get rid of connect argument here and the connection that happens in the init here ... instead, having anything that instantiates a SliderLabel worry about doing its own connections.
  • in doing that last step, get rid of the lambda here

(apologies for the crappy code...)

@Czaki
Copy link
Copy Markdown
Contributor

Czaki commented Oct 12, 2024

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
Copy link
Copy Markdown
Member Author

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
Copy link
Copy Markdown
Contributor

Czaki commented Oct 12, 2024

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

@tlambert03 tlambert03 merged commit 899c180 into pyapp-kit:main Mar 4, 2026
48 of 55 checks passed
@tlambert03 tlambert03 deleted the fixes branch March 4, 2026 15:59
@tlambert03 tlambert03 added the bug Something isn't working label Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Development

Successfully merging this pull request may close these issues.

2 participants