Skip to content
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

Add note about specifying custom base class for Exceptions. #2465

Merged
merged 3 commits into from
Sep 6, 2020

Conversation

michalsustr
Copy link
Contributor

No description provided.

Copy link
Collaborator

@YannickJadoul YannickJadoul left a comment

Choose a reason for hiding this comment

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

Thanks, @michalsustr! A few minor remarks, but good to have this documented :-)

@@ -79,6 +79,20 @@ This call creates a Python exception class with the name ``PyExp`` in the given
module and automatically converts any encountered exceptions of type ``CppExp``
into Python exceptions of type ``PyExp``.

It is possible to specify base class for the exception using the third
parameter, a pointer to `PyObject`:
Copy link
Collaborator

Choose a reason for hiding this comment

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

I know this is currently correct, but do we have any idea why this isn't a handle?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No idea, first time contributing to pybind.

Copy link
Collaborator

Choose a reason for hiding this comment

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

No problem. Let's see if @bstaletic or anyone else knows! :-)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, no clue other than "the source says so".

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, let's maybe change this in a different PR, then? (Actually, we might even change this to py::type, @henryiii?)

docs/advanced/exceptions.rst Outdated Show resolved Hide resolved
docs/advanced/exceptions.rst Show resolved Hide resolved
Copy link
Collaborator

@YannickJadoul YannickJadoul left a comment

Choose a reason for hiding this comment

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

Looks good to me, once the CI gets fixed! :-)

docs/advanced/exceptions.rst Outdated Show resolved Hide resolved
Copy link
Collaborator

@bstaletic bstaletic left a comment

Choose a reason for hiding this comment

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

Looks good.

@@ -79,6 +79,20 @@ This call creates a Python exception class with the name ``PyExp`` in the given
module and automatically converts any encountered exceptions of type ``CppExp``
into Python exceptions of type ``PyExp``.

It is possible to specify base class for the exception using the third
parameter, a pointer to `PyObject`:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Nope, no clue other than "the source says so".

@YannickJadoul
Copy link
Collaborator

No further comments, it seems, so let's merge. (If anyone still has remarks, afterwards, it's just docs and easy enough to change.)

@YannickJadoul YannickJadoul merged commit 3bd0d7a into pybind:master Sep 6, 2020
@YannickJadoul
Copy link
Collaborator

Thanks again, @michalsustr!

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.

4 participants