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 BaseExceptionGroup for Python >= 3.11 #3141

Merged
merged 1 commit into from May 8, 2023
Merged

Conversation

adriangb
Copy link
Contributor

@adriangb adriangb commented May 6, 2023

Not sure if this is totally off base, but it looks like it may be this easy to add support for ExceptionGroup?

@adamreichold
Copy link
Member

Please add a newsfragment as described in https://pyo3.rs/v0.18.3/contributing#documenting-changes including a link to an upstream changelog, PEP, etc.

The CI failure looks like this is not available in CPython 3.11.3 on Windows? Or is the name incorrect?

@adriangb
Copy link
Contributor Author

adriangb commented May 6, 2023

Please add a newsfragment

Done!

The CI failure looks like this is not available in CPython 3.11.3 on Windows? Or is the name incorrect?

Hmm not sure. I was kinda guessing at the name. It should be in 3.11 on all platforms. I think this is where it's defined? https://github.com/python/cpython/blob/f5088006ca8e9654fbc3de119462f0ab764e408b/PC/python3dll.c#L785-L786

@adamreichold
Copy link
Member

The news fragment still mentions PyExc_ExceptionGroup?

@adamreichold
Copy link
Member

I am not sure whether the test defined by the impl_native_exception! macro makes sense for BaseExceptionGroup´? Can one raise it via raise BaseExceptionGroup`?

@adriangb
Copy link
Contributor Author

adriangb commented May 6, 2023

It requires an argument raise BaseExceptionGroup("", [ValueError()]). I see there's some tests like that already.

@adriangb
Copy link
Contributor Author

adriangb commented May 6, 2023

Any suggestions on how to make sure the right Python version is being used? cargo test and such seems to be picking a non 3.11 version.

@adriangb
Copy link
Contributor Author

adriangb commented May 6, 2023

Okay, I think I figured it out.

@adriangb
Copy link
Contributor Author

adriangb commented May 6, 2023

Do we need to do anything / can we make the methods like subset available?

@adamreichold
Copy link
Member

Do we need to do anything / can we make the methods like subset available?

I think you would need to manually bind them if they are available via a C API. Are they?

If they are only available as Python methods, you can already access them via PyAny::call1 etc. and adding Rust bindings do that call1 in the background is of limited value.

@adriangb
Copy link
Contributor Author

adriangb commented May 6, 2023

I think you would need to manually bind them if they are available via a C API. Are they?

I'm not sure. I don't see any other errors that have methods, so I'm guessing none of those are exposed via the C API?

@davidhewitt
Copy link
Member

I don't see any C-API functions for BaseExceptionGroup, so I think this is probably all we would typically offer here :)

@adriangb
Copy link
Contributor Author

adriangb commented May 7, 2023

Is there anything else needed here?

@davidhewitt
Copy link
Member

I think this is fine to merge, though would you be willing to squash the commits? We like to keep a tidy commit history so bors isn't configured to squash everything

@adriangb adriangb changed the title Add ExceptionGroup for Python >= 3.11 Add BaseExceptionGroup for Python >= 3.11 May 7, 2023
@adriangb
Copy link
Contributor Author

adriangb commented May 7, 2023

Done!

@adamreichold
Copy link
Member

bors r+

@bors
Copy link
Contributor

bors bot commented May 8, 2023

Build succeeded!

The publicly hosted instance of bors-ng is deprecated and will go away soon.

If you want to self-host your own instance, instructions are here.
For more help, visit the forum.

If you want to switch to GitHub's built-in merge queue, visit their help page.

@bors bors bot merged commit 8ab3f5f into PyO3:main May 8, 2023
33 checks passed
@adriangb adriangb deleted the exception-group branch May 8, 2023 11:39
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.

None yet

3 participants