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

Added as_super and into_super methods for Bound<T: PyClass> #4351

Merged
merged 8 commits into from
Jul 14, 2024

Conversation

JRRudy1
Copy link
Contributor

@JRRudy1 JRRudy1 commented Jul 14, 2024

This PR adds two new methods to provide a more convenient mechanism for casting a Bound pyclass to its base type. The new methods mirror the corresponding methods for PyRef and PyRefMut (see #4219).

Casting Bound<T> to Bound<T::BaseType> can be useful in the context of the PyMyClassMethods idiom when an extension trait is use to implemented methods (for use from Rust) on the Bound version of a pyclass's base type. The upcast of a pyclass to its base type is a trivially safe operation, but making this cast is currently extremely verbose.

For example, there are currently two options for casting Bound<T> to Bound<T::BaseType by value:

// 3 method calls including an unnecessary `unwrap`
let sub: Bound<SubClass> = /* ... */;
let _ = sub.into_any().downcast_into::<BaseClass>().unwrap(); 
// or 2 method calls using `unsafe`
let sub: Bound<SubClass> = /* ... */;
let _ = unsafe { sub.into_any().downcast_into_unchecked::<BaseClass>() };

Upcasting by reference is a bit less painful since you can leverage Deref<Target=PyAny> to avoid the first method call, but its still ugly and requires either an unwrap or an unsafe block.

The new methods added by this PR make it much nicer:

let sub: Bound<SubClass> = /* ... */;
// upcast by reference
let _= sub.as_super(); 
// upcast by value
let _ = sub.into_super();

src/instance.rs Outdated Show resolved Hide resolved
src/instance.rs Outdated Show resolved Hide resolved
@LilyFoote LilyFoote enabled auto-merge July 14, 2024 21:45
src/instance.rs Outdated Show resolved Hide resolved
src/instance.rs Outdated Show resolved Hide resolved
@JRRudy1
Copy link
Contributor Author

JRRudy1 commented Jul 14, 2024

Good catches, thanks!

@LilyFoote LilyFoote added this pull request to the merge queue Jul 14, 2024
Merged via the queue into PyO3:main with commit d598d1f Jul 14, 2024
42 checks passed
@JRRudy1 JRRudy1 deleted the bound-as-super branch July 16, 2024 21:37
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