Skip to content

bpo-28206: Document signals Handlers, Sigmasks and Signals enums#1939

Closed
desbma wants to merge 3 commits into
python:masterfrom
desbma:issue28206
Closed

bpo-28206: Document signals Handlers, Sigmasks and Signals enums#1939
desbma wants to merge 3 commits into
python:masterfrom
desbma:issue28206

Conversation

@desbma

@desbma desbma commented Jun 4, 2017

Copy link
Copy Markdown
Contributor

Document the 3 module level IntEnum classes in signals.

See http://bugs.python.org/issue28206

https://bugs.python.org/issue28206

@mention-bot

Copy link
Copy Markdown

@desbma, thanks for your PR! By analyzing the history of the files in this pull request, we identified @birkenfeld, @loewis and @taleinat to be potential reviewers.

@berkerpeksag berkerpeksag left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

We also need to add some examples in the "Examples" section. We can use https://stackoverflow.com/a/35996948 as a starting point.

Comment thread Doc/library/signal.rst Outdated
(:const:`SIG_BLOCK`, :const:`SIG_UNBLOCK`, :const:`SIG_SETMASK`)
related constants listed below were turned into
:class:`enums <enum.IntEnum>`.
:class:`enums <enum.IntEnum>` (respectively :class:`Handlers` and :class:`Sigmasks`).

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think "respectively" should be last word here: (as Handlers and Sigmasks respectively)

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@desbma

desbma commented Aug 24, 2018

Copy link
Copy Markdown
Contributor Author
  • rebased against master
  • reworded as suggested
  • added an example

"I have made the requested changes; please review again"

@bedevere-bot

Copy link
Copy Markdown

Thanks for making the requested changes!

@berkerpeksag: please review the changes made to this pull request.

@vstinner

Copy link
Copy Markdown
Member

I removed the " needs backport to 3.6" label, the 3.6 branch no longer accept bugfixes (only security fixes are accepted): https://devguide.python.org/#status-of-python-branches

Comment thread Doc/library/signal.rst

signal.alarm(0) # Disable the alarm

:class:`enums <enum.IntEnum>` types can be used to convert from signal code to string, or the reverse::

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think there's no need to duplicate the first example here. We can enhance it instead of adding another one:

import signal, os

def handler(signum, frame):
    signame = signal.Signals(signum).name
    print(f'Signal handler called with signal {signame} ({signum})')
    raise OSError("Couldn't open device!")

# Set the signal handler and a 5-second alarm
signal.signal(signal.SIGALRM, handler)
# An enum member can also be passed as first argument
signal.signal(signal.Signals['SIGALRM'], handler)
signal.alarm(5)

# This open() may hang indefinitely
fd = os.open('/dev/ttyS0', os.O_RDWR)

signal.alarm(0)          # Disable the alarm

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Also, it would be nice to document enums at the documentation:

The signal module defines three enums:

.. class:: Signals

.. You may be able to use ``.. enum:: Signals`` instead. I don't know if it's supported now.

   Document it here.

   .. versionadded:: 3.5

@bedevere-bot

Copy link
Copy Markdown

A Python core developer has requested some changes be made to your pull request before we can consider merging it. If you could please address their requests along with any other requests in other reviews from core developers that would be appreciated.

Once you have made the requested changes, please leave a comment on this pull request containing the phrase I have made the requested changes; please review again. I will then notify any core developers who have left a review that you're ready for them to take another look at this pull request.

@desbma

desbma commented Apr 13, 2019

Copy link
Copy Markdown
Contributor Author

@berkerpeksag Since this is a very minor change, I think it'll be more efficient if you commit it directly, rather than doing back and forth with this pull request, so I am closing this.

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

Labels

awaiting changes docs Documentation in the Doc dir skip news

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants