From 2fdd52003e94c4f724c4ba06c391ac5b9489ce0d Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Mon, 18 Dec 2023 09:57:37 +0100 Subject: [PATCH 1/3] Add PyType::full_name which is tp_name and has an abi3 fallback. --- newsfragments/3660.added.md | 1 + pytests/src/misc.rs | 7 +++++++ pytests/tests/test_misc.py | 6 ++++++ src/types/typeobject.rs | 26 ++++++++++++++++++++++++++ 4 files changed, 40 insertions(+) create mode 100644 newsfragments/3660.added.md diff --git a/newsfragments/3660.added.md b/newsfragments/3660.added.md new file mode 100644 index 00000000..7350e4af --- /dev/null +++ b/newsfragments/3660.added.md @@ -0,0 +1 @@ +Added `PyType::full_name` which in contrast to `PyType::name` includes the module name. diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index b79af4b9..74770ef6 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -1,4 +1,5 @@ use pyo3::prelude::*; +use std::borrow::Cow; #[pyfunction] fn issue_219() { @@ -6,8 +7,14 @@ fn issue_219() { Python::with_gil(|_| {}); } +#[pyfunction] +fn get_type_full_name(obj: &PyAny) -> PyResult> { + obj.get_type().full_name() +} + #[pymodule] pub fn misc(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add_function(wrap_pyfunction!(issue_219, m)?)?; + m.add_function(wrap_pyfunction!(get_type_full_name, m)?)?; Ok(()) } diff --git a/pytests/tests/test_misc.py b/pytests/tests/test_misc.py index 9cc0cebc..537ee119 100644 --- a/pytests/tests/test_misc.py +++ b/pytests/tests/test_misc.py @@ -48,3 +48,9 @@ def test_import_in_subinterpreter_forbidden(): ) _xxsubinterpreters.destroy(sub_interpreter) + + +def test_type_full_name_includes_module(): + numpy = pytest.importorskip("numpy") + + assert pyo3_pytests.misc.get_type_full_name(numpy.bool_(True)) == "numpy.bool_" diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 67eeb566..74dbbfb7 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -1,5 +1,8 @@ use crate::err::{self, PyResult}; use crate::{ffi, PyAny, PyTypeInfo, Python}; +use std::borrow::Cow; +#[cfg(not(any(Py_LIMITED_API, PyPy)))] +use std::ffi::CStr; /// Represents a reference to a Python `type object`. #[repr(transparent)] @@ -35,6 +38,29 @@ impl PyType { self.getattr(intern!(self.py(), "__qualname__"))?.extract() } + /// Gets the full name, which includes the module, of the `PyType`. + pub fn full_name(&self) -> PyResult> { + #[cfg(not(any(Py_LIMITED_API, PyPy)))] + { + let name = unsafe { CStr::from_ptr((*self.as_type_ptr()).tp_name) }.to_str()?; + + Ok(Cow::Borrowed(name)) + } + + #[cfg(any(Py_LIMITED_API, PyPy))] + { + let module = self + .getattr(intern!(self.py(), "__module__"))? + .extract::<&str>()?; + + let name = self + .getattr(intern!(self.py(), "__name__"))? + .extract::<&str>()?; + + Ok(Cow::Owned(format!("{}.{}", module, name))) + } + } + /// Checks whether `self` is a subclass of `other`. /// /// Equivalent to the Python expression `issubclass(self, other)`. From 416d3c488f60257d8463aa43a7c9fdf51ccf2b69 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Tue, 19 Dec 2023 11:51:06 +0100 Subject: [PATCH 2/3] Rename name to qualname and full_name to name to better match Python --- guide/src/class/numeric.md | 2 +- guide/src/class/object.md | 4 ++-- guide/src/migration.md | 3 +++ newsfragments/3660.added.md | 1 - newsfragments/3660.changed.md | 1 + pytests/src/misc.rs | 2 +- src/err/mod.rs | 10 ++++++--- src/types/typeobject.rs | 39 ++++++++++++++++++++++++++--------- tests/test_coroutine.rs | 5 ++++- tests/test_methods.rs | 8 +++---- 10 files changed, 52 insertions(+), 23 deletions(-) delete mode 100644 newsfragments/3660.added.md create mode 100644 newsfragments/3660.changed.md diff --git a/guide/src/class/numeric.md b/guide/src/class/numeric.md index a7ad9c2c..10a06e7c 100644 --- a/guide/src/class/numeric.md +++ b/guide/src/class/numeric.md @@ -232,7 +232,7 @@ impl Number { fn __repr__(slf: &PyCell) -> PyResult { // Get the class name dynamically in case `Number` is subclassed - let class_name: &str = slf.get_type().name()?; + let class_name: String = slf.get_type().qualname()?; Ok(format!("{}({})", class_name, slf.borrow().0)) } diff --git a/guide/src/class/object.md b/guide/src/class/object.md index c6bf0483..cfaa8bb1 100644 --- a/guide/src/class/object.md +++ b/guide/src/class/object.md @@ -87,7 +87,7 @@ the subclass name. This is typically done in Python code by accessing impl Number { fn __repr__(slf: &PyCell) -> PyResult { // This is the equivalent of `self.__class__.__name__` in Python. - let class_name: &str = slf.get_type().name()?; + let class_name: String = slf.get_type().qualname()?; // To access fields of the Rust struct, we need to borrow the `PyCell`. Ok(format!("{}({})", class_name, slf.borrow().0)) } @@ -263,7 +263,7 @@ impl Number { } fn __repr__(slf: &PyCell) -> PyResult { - let class_name: &str = slf.get_type().name()?; + let class_name: String = slf.get_type().qualname()?; Ok(format!("{}({})", class_name, slf.borrow().0)) } diff --git a/guide/src/migration.md b/guide/src/migration.md index 4538f799..0c55c15c 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -74,6 +74,9 @@ Python::with_gil(|py| { }); ``` +### `PyType::name` is now `PyType::qualname` + +`PyType::name` has been renamed to `PyType::qualname` to indicate that it does indeed return the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name), matching the `__qualname__` attribute. The newly added `PyType::name` yields the full name including the module name now which corresponds to `__module__.__name__` on the level of attributes. ## from 0.19.* to 0.20 diff --git a/newsfragments/3660.added.md b/newsfragments/3660.added.md deleted file mode 100644 index 7350e4af..00000000 --- a/newsfragments/3660.added.md +++ /dev/null @@ -1 +0,0 @@ -Added `PyType::full_name` which in contrast to `PyType::name` includes the module name. diff --git a/newsfragments/3660.changed.md b/newsfragments/3660.changed.md new file mode 100644 index 00000000..8b4a3f73 --- /dev/null +++ b/newsfragments/3660.changed.md @@ -0,0 +1 @@ +`PyType::name` is now `PyType::qualname` whereas `PyType::name` efficiently accesses the full name which includes the module name. diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 74770ef6..69f3b75e 100644 --- a/pytests/src/misc.rs +++ b/pytests/src/misc.rs @@ -9,7 +9,7 @@ fn issue_219() { #[pyfunction] fn get_type_full_name(obj: &PyAny) -> PyResult> { - obj.get_type().full_name() + obj.get_type().name() } #[pymodule] diff --git a/src/err/mod.rs b/src/err/mod.rs index cd2139b6..88f930cf 100644 --- a/src/err/mod.rs +++ b/src/err/mod.rs @@ -706,7 +706,7 @@ impl std::fmt::Display for PyErr { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { Python::with_gil(|py| { let value = self.value(py); - let type_name = value.get_type().name().map_err(|_| std::fmt::Error)?; + let type_name = value.get_type().qualname().map_err(|_| std::fmt::Error)?; write!(f, "{}", type_name)?; if let Ok(s) = value.str() { write!(f, ": {}", &s.to_string_lossy()) @@ -748,7 +748,8 @@ impl PyErrArguments for PyDowncastErrorArguments { "'{}' object cannot be converted to '{}'", self.from .as_ref(py) - .name() + .qualname() + .as_deref() .unwrap_or(""), self.to ) @@ -775,7 +776,10 @@ impl<'a> std::fmt::Display for PyDowncastError<'a> { write!( f, "'{}' object cannot be converted to '{}'", - self.from.get_type().name().map_err(|_| std::fmt::Error)?, + self.from + .get_type() + .qualname() + .map_err(|_| std::fmt::Error)?, self.to ) } diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 74dbbfb7..7e92925a 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -33,13 +33,28 @@ impl PyType { py.from_borrowed_ptr(p as *mut ffi::PyObject) } - /// Gets the name of the `PyType`. - pub fn name(&self) -> PyResult<&str> { - self.getattr(intern!(self.py(), "__qualname__"))?.extract() + /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. + pub fn qualname(&self) -> PyResult { + #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))] + let name = self.getattr(intern!(self.py(), "__qualname__"))?.extract(); + + #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))] + let name = { + use crate::ffi_ptr_ext::FfiPtrExt; + use crate::types::any::PyAnyMethods; + + let obj = unsafe { + ffi::PyType_GetQualName(self.as_type_ptr()).assume_owned_or_err(self.py())? + }; + + obj.extract() + }; + + name } /// Gets the full name, which includes the module, of the `PyType`. - pub fn full_name(&self) -> PyResult> { + pub fn name(&self) -> PyResult> { #[cfg(not(any(Py_LIMITED_API, PyPy)))] { let name = unsafe { CStr::from_ptr((*self.as_type_ptr()).tp_name) }.to_str()?; @@ -49,13 +64,17 @@ impl PyType { #[cfg(any(Py_LIMITED_API, PyPy))] { - let module = self - .getattr(intern!(self.py(), "__module__"))? - .extract::<&str>()?; + let module = self.getattr(intern!(self.py(), "__module__"))?; - let name = self - .getattr(intern!(self.py(), "__name__"))? - .extract::<&str>()?; + #[cfg(not(Py_3_11))] + let name = self.getattr(intern!(self.py(), "__name__"))?; + + #[cfg(Py_3_11)] + let name = { + use crate::ffi_ptr_ext::FfiPtrExt; + + unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? } + }; Ok(Cow::Owned(format!("{}.{}", module, name))) } diff --git a/tests/test_coroutine.rs b/tests/test_coroutine.rs index 8acea3ea..206c35da 100644 --- a/tests/test_coroutine.rs +++ b/tests/test_coroutine.rs @@ -136,7 +136,10 @@ fn cancelled_coroutine() { None, ) .unwrap_err(); - assert_eq!(err.value(gil).get_type().name().unwrap(), "CancelledError"); + assert_eq!( + err.value(gil).get_type().qualname().unwrap(), + "CancelledError" + ); }) } diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 7919ac0c..6e42ad66 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -74,14 +74,14 @@ impl ClassMethod { #[classmethod] /// Test class method. fn method(cls: &PyType) -> PyResult { - Ok(format!("{}.method()!", cls.name()?)) + Ok(format!("{}.method()!", cls.qualname()?)) } #[classmethod] fn method_owned(cls: Py) -> PyResult { Ok(format!( "{}.method_owned()!", - Python::with_gil(|gil| cls.as_ref(gil).name().map(ToString::to_string))? + Python::with_gil(|gil| cls.as_ref(gil).qualname())? )) } } @@ -109,7 +109,7 @@ struct ClassMethodWithArgs {} impl ClassMethodWithArgs { #[classmethod] fn method(cls: &PyType, input: &PyString) -> PyResult { - Ok(format!("{}.method({})", cls.name()?, input)) + Ok(format!("{}.method({})", cls.qualname()?, input)) } } @@ -937,7 +937,7 @@ impl r#RawIdents { fn test_raw_idents() { Python::with_gil(|py| { let raw_idents_type = py.get_type::(); - assert_eq!(raw_idents_type.name().unwrap(), "RawIdents"); + assert_eq!(raw_idents_type.qualname().unwrap(), "RawIdents"); py_run!( py, raw_idents_type, From 68f417fb1cd09b3b576999a5fd4fcc4f669f78d4 Mon Sep 17 00:00:00 2001 From: Adam Reichold Date: Tue, 19 Dec 2023 14:59:38 +0100 Subject: [PATCH 3/3] Defend against mutable type objects when extracting their full name. --- src/types/typeobject.rs | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 7e92925a..a1a053f9 100644 --- a/src/types/typeobject.rs +++ b/src/types/typeobject.rs @@ -57,9 +57,16 @@ impl PyType { pub fn name(&self) -> PyResult> { #[cfg(not(any(Py_LIMITED_API, PyPy)))] { - let name = unsafe { CStr::from_ptr((*self.as_type_ptr()).tp_name) }.to_str()?; + let ptr = self.as_type_ptr(); - Ok(Cow::Borrowed(name)) + let name = unsafe { CStr::from_ptr((*ptr).tp_name) }.to_str()?; + + #[cfg(Py_3_10)] + if unsafe { ffi::PyType_HasFeature(ptr, ffi::Py_TPFLAGS_IMMUTABLETYPE) } != 0 { + return Ok(Cow::Borrowed(name)); + } + + Ok(Cow::Owned(name.to_owned())) } #[cfg(any(Py_LIMITED_API, PyPy))]