Revamp PyType name functions to match PEP 737 (#4196)

* Revamp PyType name functions to match PEP 737

PyType::name uses `tp_name`, which is not consistent.
[PEP 737](https://peps.python.org/pep-0737/) adds a new path forward,
so update PyType::name and add PyType::{module,fully_qualified_name}
to match the PEP.

* refactor conditional code to handle multiple Python versions better

* return `Bound<'py, str>`

* fixup

---------

Co-authored-by: David Hewitt <mail@davidhewitt.dev>

guide/src/class/numeric.md

@@ -210,7 +210,7 @@ use std::hash::{Hash, Hasher};
 use pyo3::exceptions::{PyValueError, PyZeroDivisionError};
 use pyo3::prelude::*;
 use pyo3::class::basic::CompareOp;
-use pyo3::types::PyComplex;
+use pyo3::types::{PyComplex, PyString};
 
 fn wrap(obj: &Bound<'_, PyAny>) -> PyResult<i32> {
     let val = obj.call_method1("__and__", (0xFFFFFFFF_u32,))?;
@@ -231,7 +231,7 @@ impl Number {
 
     fn __repr__(slf: &Bound<'_, Self>) -> PyResult<String> {
        // Get the class name dynamically in case `Number` is subclassed
-       let class_name: String = slf.get_type().qualname()?;
+       let class_name: Bound<'_, PyString> = slf.get_type().qualname()?;
         Ok(format!("{}({})", class_name, slf.borrow().0))
     }
 

guide/src/class/object.md

@@ -80,6 +80,7 @@ the subclass name. This is typically done in Python code by accessing
 
 ```rust
 # use pyo3::prelude::*;
+# use pyo3::types::PyString;
 #
 # #[pyclass]
 # struct Number(i32);
@@ -88,7 +89,7 @@ the subclass name. This is typically done in Python code by accessing
 impl Number {
     fn __repr__(slf: &Bound<'_, Self>) -> PyResult<String> {
         // This is the equivalent of `self.__class__.__name__` in Python.
-        let class_name: String = slf.get_type().qualname()?;
+        let class_name: Bound<'_, PyString> = slf.get_type().qualname()?;
         // To access fields of the Rust struct, we need to borrow the `PyCell`.
         Ok(format!("{}({})", class_name, slf.borrow().0))
     }
@@ -285,6 +286,7 @@ use std::hash::{Hash, Hasher};
 
 use pyo3::prelude::*;
 use pyo3::class::basic::CompareOp;
+use pyo3::types::PyString;
 
 #[pyclass]
 struct Number(i32);
@@ -297,7 +299,7 @@ impl Number {
     }
 
     fn __repr__(slf: &Bound<'_, Self>) -> PyResult<String> {
-        let class_name: String = slf.get_type().qualname()?;
+        let class_name: Bound<'_, PyString> = slf.get_type().qualname()?;
         Ok(format!("{}({})", class_name, slf.borrow().0))
     }
 

guide/src/migration.md

@@ -81,6 +81,64 @@ enum SimpleEnum {
 ```
 </details>
 
+### `PyType::name` reworked to better match Python `__name__`
+<details open>
+<summary><small>Click to expand</small></summary>
+
+This function previously would try to read directly from Python type objects' C API field (`tp_name`), in which case it
+would return a `Cow::Borrowed`. However the contents of `tp_name` don't have well-defined semantics.
+
+Instead `PyType::name()` now returns the equivalent of Python `__name__` and returns `PyResult<Bound<'py, PyString>>`.
+
+The closest equivalent to PyO3 0.21's version of `PyType::name()` has been introduced as a new function `PyType::fully_qualified_name()`,
+which is equivalent to `__module__` and `__qualname__` joined as `module.qualname`.
+
+Before:
+
+```rust,ignore
+# #![allow(deprecated, dead_code)]
+# use pyo3::prelude::*;
+# use pyo3::types::{PyBool};
+# fn main() -> PyResult<()> {
+Python::with_gil(|py| {
+    let bool_type = py.get_type_bound::<PyBool>();
+    let name = bool_type.name()?.into_owned();
+    println!("Hello, {}", name);
+
+    let mut name_upper = bool_type.name()?;
+    name_upper.to_mut().make_ascii_uppercase();
+    println!("Hello, {}", name_upper);
+
+    Ok(())
+})
+# }
+```
+
+After:
+
+```rust
+# #![allow(dead_code)]
+# use pyo3::prelude::*;
+# use pyo3::types::{PyBool};
+# fn main() -> PyResult<()> {
+Python::with_gil(|py| {
+    let bool_type = py.get_type_bound::<PyBool>();
+    let name = bool_type.name()?;
+    println!("Hello, {}", name);
+
+    // (if the full dotted path was desired, switch from `name()` to `fully_qualified_name()`)
+    let mut name_upper = bool_type.fully_qualified_name()?.to_string();
+    name_upper.make_ascii_uppercase();
+    println!("Hello, {}", name_upper);
+
+    Ok(())
+})
+# }
+```
+</details>
+
+
+
 ## from 0.20.* to 0.21
 <details>
 <summary><small>Click to expand</small></summary>

newsfragments/4196.added.md

@@ -0,0 +1,4 @@
+Add `PyType::module`, which always matches Python `__module__`.
+Add `PyType::fully_qualified_name` which matches the "fully qualified name"
+defined in https://peps.python.org/pep-0737 (not exposed in Python),
+which is useful for error messages and `repr()` implementations.

newsfragments/4196.changed.md

@@ -0,0 +1 @@
+Change `PyType::name` to always match Python `__name__`.

pyo3-ffi/src/object.rs

@@ -261,6 +261,14 @@ extern "C" {
     #[cfg_attr(PyPy, link_name = "PyPyType_GetQualName")]
     pub fn PyType_GetQualName(arg1: *mut PyTypeObject) -> *mut PyObject;
 
+    #[cfg(Py_3_13)]
+    #[cfg_attr(PyPy, link_name = "PyPyType_GetFullyQualifiedName")]
+    pub fn PyType_GetFullyQualifiedName(arg1: *mut PyTypeObject) -> *mut PyObject;
+
+    #[cfg(Py_3_13)]
+    #[cfg_attr(PyPy, link_name = "PyPyType_GetModuleName")]
+    pub fn PyType_GetModuleName(arg1: *mut PyTypeObject) -> *mut PyObject;
+
     #[cfg(Py_3_12)]
     #[cfg_attr(PyPy, link_name = "PyPyType_FromMetaclass")]
     pub fn PyType_FromMetaclass(

pytests/src/misc.rs

@@ -1,5 +1,7 @@
-use pyo3::{prelude::*, types::PyDict};
-use std::borrow::Cow;
+use pyo3::{
+    prelude::*,
+    types::{PyDict, PyString},
+};
 
 #[pyfunction]
 fn issue_219() {
@@ -8,8 +10,8 @@ fn issue_219() {
 }
 
 #[pyfunction]
-fn get_type_full_name(obj: &Bound<'_, PyAny>) -> PyResult<String> {
-    obj.get_type().name().map(Cow::into_owned)
+fn get_type_fully_qualified_name<'py>(obj: &Bound<'py, PyAny>) -> PyResult<Bound<'py, PyString>> {
+    obj.get_type().fully_qualified_name()
 }
 
 #[pyfunction]
@@ -33,7 +35,7 @@ fn get_item_and_run_callback(dict: Bound<'_, PyDict>, callback: Bound<'_, PyAny>
 #[pymodule]
 pub fn misc(m: &Bound<'_, PyModule>) -> PyResult<()> {
     m.add_function(wrap_pyfunction!(issue_219, m)?)?;
-    m.add_function(wrap_pyfunction!(get_type_full_name, m)?)?;
+    m.add_function(wrap_pyfunction!(get_type_fully_qualified_name, m)?)?;
     m.add_function(wrap_pyfunction!(accepts_bool, m)?)?;
     m.add_function(wrap_pyfunction!(get_item_and_run_callback, m)?)?;
     Ok(())

pytests/tests/test_misc.py

@@ -51,11 +51,11 @@ def test_import_in_subinterpreter_forbidden():
     subinterpreters.destroy(sub_interpreter)
 
 
-def test_type_full_name_includes_module():
+def test_type_fully_qualified_name_includes_module():
     numpy = pytest.importorskip("numpy")
 
     # For numpy 1.x and 2.x
-    assert pyo3_pytests.misc.get_type_full_name(numpy.bool_(True)) in [
+    assert pyo3_pytests.misc.get_type_fully_qualified_name(numpy.bool_(True)) in [
         "numpy.bool",
         "numpy.bool_",
     ]

src/err/mod.rs

@@ -971,16 +971,13 @@ struct PyDowncastErrorArguments {
 
 impl PyErrArguments for PyDowncastErrorArguments {
     fn arguments(self, py: Python<'_>) -> PyObject {
-        format!(
-            "'{}' object cannot be converted to '{}'",
-            self.from
-                .bind(py)
-                .qualname()
-                .as_deref()
-                .unwrap_or("<failed to extract type name>"),
-            self.to
-        )
-        .to_object(py)
+        const FAILED_TO_EXTRACT: Cow<'_, str> = Cow::Borrowed("<failed to extract type name>");
+        let from = self.from.bind(py).qualname();
+        let from = match &from {
+            Ok(qn) => qn.to_cow().unwrap_or(FAILED_TO_EXTRACT),
+            Err(_) => FAILED_TO_EXTRACT,
+        };
+        format!("'{}' object cannot be converted to '{}'", from, self.to).to_object(py)
     }
 }
 

src/types/boolobject.rs

@@ -111,11 +111,15 @@ impl FromPyObject<'_> for bool {
             Err(err) => err,
         };
 
-        if obj
-            .get_type()
+        let is_numpy_bool = {
+            let ty = obj.get_type();
+            ty.module().map_or(false, |module| module == "numpy")
+                && ty
                     .name()
-            .map_or(false, |name| name == "numpy.bool_" || name == "numpy.bool")
-        {
+                    .map_or(false, |name| name == "bool_" || name == "bool")
+        };
+
+        if is_numpy_bool {
             let missing_conversion = |obj: &Bound<'_, PyAny>| {
                 PyTypeError::new_err(format!(
                     "object of type '{}' does not define a '__bool__' conversion",

src/types/typeobject.rs

@@ -1,13 +1,14 @@
 use crate::err::{self, PyResult};
 use crate::instance::Borrowed;
+#[cfg(not(Py_3_13))]
+use crate::pybacked::PyBackedStr;
 use crate::types::any::PyAnyMethods;
 use crate::types::PyTuple;
 #[cfg(feature = "gil-refs")]
 use crate::PyNativeType;
 use crate::{ffi, Bound, PyAny, PyTypeInfo, Python};
-use std::borrow::Cow;
-#[cfg(not(any(Py_LIMITED_API, PyPy)))]
-use std::ffi::CStr;
+
+use super::PyString;
 /// Represents a reference to a Python `type object`.
 #[repr(transparent)]
 pub struct PyType(PyAny);
@@ -71,16 +72,19 @@ impl PyType {
         Self::from_borrowed_type_ptr(py, p).into_gil_ref()
     }
 
-    /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
-    pub fn qualname(&self) -> PyResult<String> {
-        self.as_borrowed().qualname()
+    /// Gets the name of the `PyType`. Equivalent to `self.__name__` in Python.
+    pub fn name(&self) -> PyResult<&PyString> {
+        self.as_borrowed().name().map(Bound::into_gil_ref)
     }
 
-    /// Gets the full name, which includes the module, of the `PyType`.
-    pub fn name(&self) -> PyResult<Cow<'_, str>> {
-        self.as_borrowed().name()
+    /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
+    /// Equivalent to `self.__qualname__` in Python.
+    pub fn qualname(&self) -> PyResult<&PyString> {
+        self.as_borrowed().qualname().map(Bound::into_gil_ref)
     }
 
+    // `module` and `fully_qualified_name` intentionally omitted
+
     /// Checks whether `self` is a subclass of `other`.
     ///
     /// Equivalent to the Python expression `issubclass(self, other)`.
@@ -110,11 +114,18 @@ pub trait PyTypeMethods<'py>: crate::sealed::Sealed {
     /// Retrieves the underlying FFI pointer associated with this Python object.
     fn as_type_ptr(&self) -> *mut ffi::PyTypeObject;
 
-    /// Gets the full name, which includes the module, of the `PyType`.
-    fn name(&self) -> PyResult<Cow<'_, str>>;
+    /// Gets the name of the `PyType`. Equivalent to `self.__name__` in Python.
+    fn name(&self) -> PyResult<Bound<'py, PyString>>;
 
     /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
-    fn qualname(&self) -> PyResult<String>;
+    /// Equivalent to `self.__qualname__` in Python.
+    fn qualname(&self) -> PyResult<Bound<'py, PyString>>;
+
+    /// Gets the name of the module defining the `PyType`.
+    fn module(&self) -> PyResult<Bound<'py, PyString>>;
+
+    /// Gets the [fully qualified name](https://peps.python.org/pep-0737/#add-pytype-getfullyqualifiedname-function) of the `PyType`.
+    fn fully_qualified_name(&self) -> PyResult<Bound<'py, PyString>>;
 
     /// Checks whether `self` is a subclass of `other`.
     ///
@@ -148,25 +159,82 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
     }
 
     /// Gets the name of the `PyType`.
-    fn name(&self) -> PyResult<Cow<'_, str>> {
-        Borrowed::from(self).name()
+    fn name(&self) -> PyResult<Bound<'py, PyString>> {
+        #[cfg(not(Py_3_11))]
+        let name = self
+            .getattr(intern!(self.py(), "__name__"))?
+            .downcast_into()?;
+
+        #[cfg(Py_3_11)]
+        let name = unsafe {
+            use crate::ffi_ptr_ext::FfiPtrExt;
+            ffi::PyType_GetName(self.as_type_ptr())
+                .assume_owned_or_err(self.py())?
+                // SAFETY: setting `__name__` from Python is required to be a `str`
+                .downcast_into_unchecked()
+        };
+
+        Ok(name)
     }
 
-    fn qualname(&self) -> PyResult<String> {
-        #[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))]
-        let name = self.getattr(intern!(self.py(), "__qualname__"))?.extract();
+    /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
+    fn qualname(&self) -> PyResult<Bound<'py, PyString>> {
+        #[cfg(not(Py_3_11))]
+        let name = self
+            .getattr(intern!(self.py(), "__qualname__"))?
+            .downcast_into()?;
 
-        #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))]
-        let name = {
+        #[cfg(Py_3_11)]
+        let name = unsafe {
             use crate::ffi_ptr_ext::FfiPtrExt;
-            let obj = unsafe {
-                ffi::PyType_GetQualName(self.as_type_ptr()).assume_owned_or_err(self.py())?
+            ffi::PyType_GetQualName(self.as_type_ptr())
+                .assume_owned_or_err(self.py())?
+                // SAFETY: setting `__qualname__` from Python is required to be a `str`
+                .downcast_into_unchecked()
         };
 
-            obj.extract()
+        Ok(name)
+    }
+
+    /// Gets the name of the module defining the `PyType`.
+    fn module(&self) -> PyResult<Bound<'py, PyString>> {
+        #[cfg(not(Py_3_13))]
+        let name = self.getattr(intern!(self.py(), "__module__"))?;
+
+        #[cfg(Py_3_13)]
+        let name = unsafe {
+            use crate::ffi_ptr_ext::FfiPtrExt;
+            ffi::PyType_GetModuleName(self.as_type_ptr()).assume_owned_or_err(self.py())?
         };
 
-        name
+        // `__module__` is never guaranteed to be a `str`
+        name.downcast_into().map_err(Into::into)
+    }
+
+    /// Gets the [fully qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
+    fn fully_qualified_name(&self) -> PyResult<Bound<'py, PyString>> {
+        #[cfg(not(Py_3_13))]
+        let name = {
+            let module = self.getattr(intern!(self.py(), "__module__"))?;
+            let qualname = self.getattr(intern!(self.py(), "__qualname__"))?;
+
+            let module_str = module.extract::<PyBackedStr>()?;
+            if module_str == "builtins" || module_str == "__main__" {
+                qualname.downcast_into()?
+            } else {
+                PyString::new_bound(self.py(), &format!("{}.{}", module, qualname))
+            }
+        };
+
+        #[cfg(Py_3_13)]
+        let name = unsafe {
+            use crate::ffi_ptr_ext::FfiPtrExt;
+            ffi::PyType_GetFullyQualifiedName(self.as_type_ptr())
+                .assume_owned_or_err(self.py())?
+                .downcast_into_unchecked()
+        };
+
+        Ok(name)
     }
 
     /// Checks whether `self` is a subclass of `other`.
@@ -232,43 +300,11 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
     }
 }
 
-impl<'a> Borrowed<'a, '_, PyType> {
-    fn name(self) -> PyResult<Cow<'a, str>> {
-        #[cfg(not(any(Py_LIMITED_API, PyPy)))]
-        {
-            let ptr = self.as_type_ptr();
-
-            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))]
-        {
-            let module = self.getattr(intern!(self.py(), "__module__"))?;
-
-            #[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)))
-        }
-    }
-}
-
 #[cfg(test)]
 mod tests {
-    use crate::types::{PyAnyMethods, PyBool, PyInt, PyLong, PyTuple, PyTypeMethods};
+    use crate::types::{
+        PyAnyMethods, PyBool, PyInt, PyLong, PyModule, PyTuple, PyType, PyTypeMethods,
+    };
     use crate::PyAny;
     use crate::Python;
 
@@ -330,4 +366,72 @@ mod tests {
                 .unwrap());
         });
     }
+
+    #[test]
+    fn test_type_names_standard() {
+        Python::with_gil(|py| {
+            let module = PyModule::from_code_bound(
+                py,
+                r#"
+class MyClass:
+    pass
+"#,
+                file!(),
+                "test_module",
+            )
+            .expect("module create failed");
+
+            let my_class = module.getattr("MyClass").unwrap();
+            let my_class_type = my_class.downcast_into::<PyType>().unwrap();
+            assert_eq!(my_class_type.name().unwrap(), "MyClass");
+            assert_eq!(my_class_type.qualname().unwrap(), "MyClass");
+            assert_eq!(my_class_type.module().unwrap(), "test_module");
+            assert_eq!(
+                my_class_type.fully_qualified_name().unwrap(),
+                "test_module.MyClass"
+            );
+        });
+    }
+
+    #[test]
+    fn test_type_names_builtin() {
+        Python::with_gil(|py| {
+            let bool_type = py.get_type_bound::<PyBool>();
+            assert_eq!(bool_type.name().unwrap(), "bool");
+            assert_eq!(bool_type.qualname().unwrap(), "bool");
+            assert_eq!(bool_type.module().unwrap(), "builtins");
+            assert_eq!(bool_type.fully_qualified_name().unwrap(), "bool");
+        });
+    }
+
+    #[test]
+    fn test_type_names_nested() {
+        Python::with_gil(|py| {
+            let module = PyModule::from_code_bound(
+                py,
+                r#"
+class OuterClass:
+    class InnerClass:
+        pass
+"#,
+                file!(),
+                "test_module",
+            )
+            .expect("module create failed");
+
+            let outer_class = module.getattr("OuterClass").unwrap();
+            let inner_class = outer_class.getattr("InnerClass").unwrap();
+            let inner_class_type = inner_class.downcast_into::<PyType>().unwrap();
+            assert_eq!(inner_class_type.name().unwrap(), "InnerClass");
+            assert_eq!(
+                inner_class_type.qualname().unwrap(),
+                "OuterClass.InnerClass"
+            );
+            assert_eq!(inner_class_type.module().unwrap(), "test_module");
+            assert_eq!(
+                inner_class_type.fully_qualified_name().unwrap(),
+                "test_module.OuterClass.InnerClass"
+            );
+        });
+    }
 }

tests/test_methods.rs

@@ -78,9 +78,8 @@ impl ClassMethod {
     }
 
     #[classmethod]
-    fn method_owned(cls: Py<PyType>) -> PyResult<String> {
-        let qualname = Python::with_gil(|gil| cls.bind(gil).qualname())?;
-        Ok(format!("{}.method_owned()!", qualname))
+    fn method_owned(cls: Py<PyType>, py: Python<'_>) -> PyResult<String> {
+        Ok(format!("{}.method_owned()!", cls.bind(py).qualname()?))
     }
 }