-
-
Notifications
You must be signed in to change notification settings - Fork 30.9k
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
bpo-33332: Add signal.valid_signals() #6581
Conversation
ecc75a0
to
94eb990
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It would be interesting to also implement the feature on Windows where "valid signals" is hard to guess for a Unix user. See signal_signal() of Modules/signalmodule.c for valid signals: I count 6 signals, 7 if SIGBREAK is also available.
Lib/test/test_signal.py
Outdated
self.assertIsInstance(s, set) | ||
self.assertIn(signal.Signals.SIGINT, s) | ||
self.assertNotIn(0, s) | ||
self.assertNotIn(signal.NSIG, s) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of testing len(s) < NSIG? Maybe len(s) <= NSIG?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure it's very useful.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The purpose is to test if NSIG is consistent with sigfillset(), to check if something is really wrong on a platform. I would like to know if range(0, NSIG) miss some signals on some platforms :-)
You may also modify:
|
Thanks for the comments. You're right about Windows. I'll also fix asyncio and test.support. |
f505b3f
to
1b694b3
Compare
Big +1 for this feature. Can you make it return a |
Frozen sets are nice if you want to hash them, otherwise I don't really see the point. |
6318e94
to
4b52abf
Compare
What's the point of being able to modify the returned set? ;) If you return a frozenset you can cache it (something that I would recommend doing in this PR anyways). This isn't a deal breaker, it's just that I would use a frozen set here. Overall LGTM. |
4b52abf
to
ed4405c
Compare
ed4405c
to
e7c0fd2
Compare
Other functions like |
First I wanted to get a list, but then I read the PR and I like reusing existing code :-) If someone wants an immutable type, it's very simple: frozenset(signal.valid_signals()). An immutable type might be justified if it was a module constant, but it's a function which creates a new object at each call, so IMHO a set is just fine. I don't want to modify other signal functions just to be pedantic. I like Antoine's rationale to use set in valid_signals(). |
https://bugs.python.org/issue33332