Skip to content

bpo-19225: Lack of c api exceptions doc#881

Merged
serhiy-storchaka merged 7 commits into
python:masterfrom
cocoatomo:lack-of-c-api-exceptions-doc
Apr 2, 2017
Merged

bpo-19225: Lack of c api exceptions doc#881
serhiy-storchaka merged 7 commits into
python:masterfrom
cocoatomo:lack-of-c-api-exceptions-doc

Conversation

@cocoatomo

Copy link
Copy Markdown
Contributor

see bpo-19225.

I create this patch based on the patch created by Tamás Bence Gedai (beng94).

@mention-bot

Copy link
Copy Markdown

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

Comment thread Doc/c-api/exceptions.rst
| :c:data:`PyExc_BaseException` | :exc:`BaseException` | \(1) |
+-----------------------------------------+---------------------------------+----------+
| :c:data:`PyExc_Exception` | :exc:`Exception` | \(1) |
+-----------------------------------------+---------------------------------+----------+

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 feel BaseException and Exception are special enough to ignore alphabetical order.

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.

And the same is true for Warning.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

Comment thread Doc/c-api/exceptions.rst
+-----------------------------------------+---------------------------------+----------+
| :c:data:`PyExc_Exception` | :exc:`Exception` | \(1) |
+-----------------------------------------+---------------------------------+----------+
| :c:data:`PyExc_ArithmeticError` | :exc:`ArithmeticError` | \(1) |

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.

Since that are definitions of PyExc_* names, wouldn't be better to use the c:var directive?

.. c:var:: PyExc_ArithmeticError

Perhaps the index directive below will be not needed after that.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I think this table is the most suitable place for definition of PyExc_* names.
I would fix these markup and delete index directives.

Comment thread Doc/c-api/exceptions.rst
| :c:data:`PyExc_BaseException` | :exc:`BaseException` | \(1) |
+-----------------------------------------+---------------------------------+----------+
| :c:data:`PyExc_Exception` | :exc:`Exception` | \(1) |
+-----------------------------------------+---------------------------------+----------+

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.

And the same is true for Warning.

Comment thread Doc/c-api/exceptions.rst Outdated
Notes:

(1)
This is a base class for other standard exceptions.

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.

... for other standard warning categories.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

👍

@cocoatomo

Copy link
Copy Markdown
Contributor Author

Thanks for your good advisement. I updated the pull request.

@MSeifert04

MSeifert04 commented Mar 29, 2017

Copy link
Copy Markdown
Contributor

The tables will be really huge with the .. c:var: PyExc[...] directives. I tried it a few days ago but haven't suggested it because it almost doubled the vertical extend (compared to the current formatting) of the table:

unbenannt

@serhiy-storchaka

Copy link
Copy Markdown
Member

Hmm, then it would be better to restore the variant with :c:data: and separate .. index::.

@cocoatomo

Copy link
Copy Markdown
Contributor Author

I changed .. c:var:: back to :c:data:and restore .. index:: in the more appropriate position.

@brettcannon brettcannon added the docs Documentation in the Doc dir label Mar 30, 2017
@cocoatomo

Copy link
Copy Markdown
Contributor Author

@methane @serhiy-storchaka @MSeifert04
Is any additional work needed for this pull request?

@serhiy-storchaka

Copy link
Copy Markdown
Member

Move PyExc_BaseException, PyExc_Exception and PyExc_Warning at the top of corresponding tables.

@cocoatomo

Copy link
Copy Markdown
Contributor Author

Thanks for your comment.
I made a mistake at the commit a0e3bb7.

@serhiy-storchaka
serhiy-storchaka merged commit e8c7631 into python:master Apr 2, 2017
Mariatta pushed a commit to Mariatta/cpython that referenced this pull request Apr 3, 2017
…ames in C API doc (pythonGH-881)

Move the `.. index` directive to more appropriate place.
(cherry picked from commit e8c7631)
Mariatta pushed a commit to Mariatta/cpython that referenced this pull request Apr 3, 2017
…ames in C API doc (pythonGH-881)

Move the `.. index` directive to more appropriate place..
(cherry picked from commit e8c7631)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

docs Documentation in the Doc dir

Projects

None yet

Development

Successfully merging this pull request may close these issues.

9 participants