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.

Sign up for GitHub

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

Jump to bottom

Added support for PyErr_WriteUnraisable #2889

Merged
merged 1 commit into from
Jan 29, 2023
Merged

Added support for PyErr_WriteUnraisable #2889

merged 1 commit into from
Jan 29, 2023

Conversation

mitsuhiko
Copy link
Contributor

Fixes #2884

davidhewitt
davidhewitt reviewed Jan 18, 2023
View reviewed changes
Copy link
Member

@davidhewitt davidhewitt left a comment

Choose a reason for hiding this comment

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

Thanks! New API and test look fine to me, though I have some nits on the test code :)

Willing to fixup those and squash to a single commit? Then let's merge 👍

tests/test_exceptions.rs Outdated Show resolved Hide resolved
tests/test_exceptions.rs Outdated Show resolved Hide resolved
tests/test_exceptions.rs Outdated Show resolved Hide resolved
tests/test_exceptions.rs Outdated Show resolved Hide resolved
@mitsuhiko mitsuhiko force-pushed the feature/write-unraisable branch from f4197d3 to 8dd78f2 Compare January 18, 2023 21:12
mejrs
mejrs reviewed Jan 18, 2023
View reviewed changes
src/err/mod.rs Show resolved Hide resolved
@mitsuhiko mitsuhiko force-pushed the feature/write-unraisable branch 2 times, most recently from 22c02f0 to b70e302 Compare January 18, 2023 21:29
@davidhewitt davidhewitt mentioned this pull request Jan 19, 2023
Open
davidhewitt
davidhewitt reviewed Jan 19, 2023
View reviewed changes
newsfragments/2884.added.md Outdated Show resolved Hide resolved
tests/test_exceptions.rs Show resolved Hide resolved
@mitsuhiko mitsuhiko force-pushed the feature/write-unraisable branch from b70e302 to 4a60cc4 Compare January 21, 2023 10:34
@davidhewitt
Copy link
Member

@mitsuhiko thanks again for this - I'll give it a quick rebase & fixup this morning so that hopefully it passes CI and merges. 👍

@davidhewitt davidhewitt force-pushed the feature/write-unraisable branch 2 times, most recently from b70e302 to ee0794d Compare January 27, 2023 08:23
@davidhewitt
Copy link
Member

bors r+

bors bot added a commit that referenced this pull request Jan 27, 2023
@bors @alexpyattaev
@davidhewitt @mitsuhiko
Merge #2873 #2889 #2923 #2924
e9a29ef 
2873: A new example that shows how to integrate Python plugins that use pyclasses into a Rust app r=davidhewitt a=alexpyattaev

Example showing integration of a Python plugin into a Rust app while having option to test pyclass based API without the main app.  This also illustrates some aspects related to import of Python modules into a Rust app while also having an API module available for the Python code to be able to produce Rust objects. 

CI seems to fail on my local machine for reasons unrelated to the example just added:
```
error: unused macro definition: `check_struct`
  --> pyo3-ffi-check/src/main.rs:13:18
   |
13 |     macro_rules! check_struct {
   |                  ^^^^^^^^^^^^
   |
```

2889: Added support for PyErr_WriteUnraisable r=davidhewitt a=mitsuhiko

Fixes #2884

2923: hygiene: fix `#[pymethods(crate = "...")]` r=davidhewitt a=davidhewitt

Got to the bottom of the hygiene issue in test of #2914 

Turns out that `#[pymethods] #[pyo3(crate = "...")]` works, but `#[pymethods(crate = "...")]` was ignoring the argument.

Added a tweak to fix this and a snippet in the hygiene test (which fails on `main`). 

2924: remove unneeded into_iter calls r=davidhewitt a=davidhewitt

Clippy complaining about these to me this morning locally.

Co-authored-by: Alex Pyattaev <[email protected]>
Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Armin Ronacher <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 27, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Jan 27, 2023
@bors @alexpyattaev
@davidhewitt @mitsuhiko
Merge #2873 #2889
5631d68 
2873: A new example that shows how to integrate Python plugins that use pyclasses into a Rust app r=davidhewitt a=alexpyattaev

Example showing integration of a Python plugin into a Rust app while having option to test pyclass based API without the main app.  This also illustrates some aspects related to import of Python modules into a Rust app while also having an API module available for the Python code to be able to produce Rust objects. 

CI seems to fail on my local machine for reasons unrelated to the example just added:
```
error: unused macro definition: `check_struct`
  --> pyo3-ffi-check/src/main.rs:13:18
   |
13 |     macro_rules! check_struct {
   |                  ^^^^^^^^^^^^
   |
```

2889: Added support for PyErr_WriteUnraisable r=davidhewitt a=mitsuhiko

Fixes #2884

Co-authored-by: Alex Pyattaev <[email protected]>
Co-authored-by: David Hewitt <[email protected]>
Co-authored-by: Armin Ronacher <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 27, 2023

Build failed (retrying...):

bors bot added a commit that referenced this pull request Jan 27, 2023
@bors @mitsuhiko
Merge #2889
ece1186 
2889: Added support for PyErr_WriteUnraisable r=davidhewitt a=mitsuhiko

Fixes #2884

Co-authored-by: Armin Ronacher <[email protected]>
@bors
Copy link
Contributor

bors bot commented Jan 27, 2023

Build failed:

@davidhewitt davidhewitt mentioned this pull request Jan 29, 2023
Closed
@davidhewitt davidhewitt force-pushed the feature/write-unraisable branch from ee0794d to 066880e Compare January 29, 2023 20:01
@davidhewitt
Copy link
Member

I've rebased and rewritten the test to not use py.run.

@davidhewitt
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented Jan 29, 2023

Build succeeded:

  • conclusion

@bors bors bot merged commit cb38ff0 into PyO3:main Jan 29, 2023
@mitsuhiko mitsuhiko deleted the feature/write-unraisable branch January 29, 2023 22:06
@messense messense mentioned this pull request Feb 8, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mejrs mejrs mejrs left review comments

@davidhewitt davidhewitt davidhewitt left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Expose PyErr_WriteUnraisable
3 participants
@mitsuhiko @davidhewitt @mejrs