Skip to content

Conversation

@sldesnoo-Delft
Copy link
Contributor

DelayedKeyboardInterrupt does not work with Spyder 6.0, because Spyder already installs its own handler for SIGINT. Other environment might do the same.

The modified version checks if the installed interrupt handler is from the DelayedKeyboardInterrupt. If not it installs a new handler.

Automatic test generating KeyboardInterrupt is not straightforward possible.

@sldesnoo-Delft sldesnoo-Delft requested a review from a team as a code owner May 21, 2025 14:54
@codecov
Copy link

codecov bot commented May 22, 2025

Codecov Report

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

Project coverage is 58.74%. Comparing base (464dd78) to head (7e0abe6).
Report is 6 commits behind head on main.

Files with missing lines Patch % Lines
src/qcodes/utils/delaykeyboardinterrupt.py 77.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7163      +/-   ##
==========================================
- Coverage   59.91%   58.74%   -1.18%     
==========================================
  Files         342      342              
  Lines       31467    31242     -225     
==========================================
- Hits        18855    18354     -501     
- Misses      12612    12888     +276     

☔ 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.

@jenshnielsen
Copy link
Collaborator

@sldesnoo-Delft Sorry for the slow response. If I understand this correctly the idea of this pr is to make sure that the signal handler is always replaced? Originally I wrote the code such that it would only replace the handler if the default one was in place. This was an attempt to not break other libraries handlers but perhaps that is not the best approach?

@sldesnoo-Delft
Copy link
Contributor Author

Yes, indeed. The problem is that Spyder 6 already replaces the interrupt handler.
In our lab we use the DelayedKeyboardInterrupt to protect communication with hardware. It prevents that the request/response process gets interrupted resulting in failures on the next request.
With Spyder 6 we suddenly saw strange communication failures after keyboard interrupts.
Other environments might also replace the interrupt handler.

My current solution is to use our own simple version of the DelayedKeyboardInterrupt to protect the communication. But I think it would be nice if the QCoDeS implementation would also work in these situations.

@jenshnielsen
Copy link
Collaborator

@sldesnoo-Delft Makes sense. What do you think about implementing your pr as it currently is (fixing the small type checking issues and precommit) but then adding a config option to the qcodes config to allow users to disable this in case they really want the other handler?

@sldesnoo-Delft
Copy link
Contributor Author

That sounds like a good addition. It's good to have an option to get the old behaviour.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants