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

feature gate deprecated APIs for PyErr and exceptions #4136

Merged
merged 1 commit into from Apr 30, 2024
Merged

Conversation

Icxolu
Copy link
Contributor

@Icxolu Icxolu commented Apr 29, 2024

Part of #3960

Move deprecated PyErr and exception APIs behind gil-refs features gate.

@Icxolu Icxolu added the CI-skip-changelog Skip checking changelog entry label Apr 29, 2024
@@ -1068,6 +1071,7 @@ mod tests {
}

#[test]
#[cfg(feature = "gil-refs")]
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to convert this test to use bound? It looks like into_ref() is the only deprecated piece here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This explicitly tests the std::error::Error impl, which the Bound api can't implement anymore.

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, do we need to be providing an impl of Error for Bound<'_, T>?

I think I missed this because this PR doesn't (yet?) but that impl behind a cfg, unless I really missed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The Error impl does not exist anymore, because we can't implement source() anymore using the Bound API, see #3820 (comment). (It would also be problematic for user defined exceptions, because of orphan rules, but I think we could work around that using a marker trait and blanket impl if we wanted to)

Copy link
Contributor

@alex alex left a comment

Choose a reason for hiding this comment

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

One q, otherwise this lgtm

@alex alex added this pull request to the merge queue Apr 30, 2024
Merged via the queue into PyO3:main with commit 22c5cff Apr 30, 2024
43 of 44 checks passed
@Icxolu Icxolu deleted the pyerr branch April 30, 2024 18:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI-skip-changelog Skip checking changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants