diff --git a/guide/src/class/numeric.md b/guide/src/class/numeric.md index 361d2fb6d36..20a1a041450 100644 --- a/guide/src/class/numeric.md +++ b/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 { let val = obj.call_method1("__and__", (0xFFFFFFFF_u32,))?; @@ -231,7 +231,7 @@ impl Number { fn __repr__(slf: &Bound<'_, Self>) -> PyResult { // 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)) } diff --git a/guide/src/class/object.md b/guide/src/class/object.md index c0d25cd0597..e9ea549aab4 100644 --- a/guide/src/class/object.md +++ b/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 { // 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 { - let class_name: String = slf.get_type().qualname()?; + let class_name: Bound<'_, PyString> = slf.get_type().qualname()?; Ok(format!("{}({})", class_name, slf.borrow().0)) } diff --git a/guide/src/migration.md b/guide/src/migration.md index 77f97cf01d2..8ac3ff16c47 100644 --- a/guide/src/migration.md +++ b/guide/src/migration.md @@ -81,6 +81,64 @@ enum SimpleEnum { ``` +### `PyType::name` reworked to better match Python `__name__` +
+Click to expand + +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>`. + +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::(); + 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::(); + 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(()) +}) +# } +``` +
+ + + ## from 0.20.* to 0.21
Click to expand diff --git a/newsfragments/4196.added.md b/newsfragments/4196.added.md new file mode 100644 index 00000000000..d9c295d551c --- /dev/null +++ b/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. diff --git a/newsfragments/4196.changed.md b/newsfragments/4196.changed.md new file mode 100644 index 00000000000..4ea69180a2d --- /dev/null +++ b/newsfragments/4196.changed.md @@ -0,0 +1 @@ +Change `PyType::name` to always match Python `__name__`. diff --git a/pyo3-ffi/src/object.rs b/pyo3-ffi/src/object.rs index b33ee558a37..7acd0897217 100644 --- a/pyo3-ffi/src/object.rs +++ b/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( diff --git a/pytests/src/misc.rs b/pytests/src/misc.rs index 7704098bd5b..ed9c9333ec2 100644 --- a/pytests/src/misc.rs +++ b/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 { - obj.get_type().name().map(Cow::into_owned) +fn get_type_fully_qualified_name<'py>(obj: &Bound<'py, PyAny>) -> PyResult> { + 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(()) diff --git a/pytests/tests/test_misc.py b/pytests/tests/test_misc.py index fc8e1095705..7f43fbf11e0 100644 --- a/pytests/tests/test_misc.py +++ b/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_", ] diff --git a/src/err/mod.rs b/src/err/mod.rs index 6bfe1a6cc99..205145d4e15 100644 --- a/src/err/mod.rs +++ b/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(""), - self.to - ) - .to_object(py) + const FAILED_TO_EXTRACT: Cow<'_, str> = Cow::Borrowed(""); + 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) } } diff --git a/src/types/boolobject.rs b/src/types/boolobject.rs index 52465ef305f..04c1fd4c113 100644 --- a/src/types/boolobject.rs +++ b/src/types/boolobject.rs @@ -111,11 +111,15 @@ impl FromPyObject<'_> for bool { Err(err) => err, }; - if obj - .get_type() - .name() - .map_or(false, |name| name == "numpy.bool_" || name == "numpy.bool") - { + let is_numpy_bool = { + let ty = obj.get_type(); + ty.module().map_or(false, |module| module == "numpy") + && ty + .name() + .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", diff --git a/src/types/typeobject.rs b/src/types/typeobject.rs index 9c2d5c5f2c4..9638a2731a3 100644 --- a/src/types/typeobject.rs +++ b/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 { - 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> { - 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>; + /// Gets the name of the `PyType`. Equivalent to `self.__name__` in Python. + fn name(&self) -> PyResult>; /// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`. - fn qualname(&self) -> PyResult; + /// Equivalent to `self.__qualname__` in Python. + fn qualname(&self) -> PyResult>; + + /// Gets the name of the module defining the `PyType`. + fn module(&self) -> PyResult>; + + /// Gets the [fully qualified name](https://peps.python.org/pep-0737/#add-pytype-getfullyqualifiedname-function) of the `PyType`. + fn fully_qualified_name(&self) -> PyResult>; /// 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> { - Borrowed::from(self).name() + fn name(&self) -> PyResult> { + #[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 { - #[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> { + #[cfg(not(Py_3_11))] + let name = self + .getattr(intern!(self.py(), "__qualname__"))? + .downcast_into()?; + + #[cfg(Py_3_11)] + let name = unsafe { + use crate::ffi_ptr_ext::FfiPtrExt; + 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() + }; - #[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))] - let name = { + Ok(name) + } + + /// Gets the name of the module defining the `PyType`. + fn module(&self) -> PyResult> { + #[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; - let obj = unsafe { - ffi::PyType_GetQualName(self.as_type_ptr()).assume_owned_or_err(self.py())? - }; + ffi::PyType_GetModuleName(self.as_type_ptr()).assume_owned_or_err(self.py())? + }; - obj.extract() + // `__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> { + #[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::()?; + if module_str == "builtins" || module_str == "__main__" { + qualname.downcast_into()? + } else { + PyString::new_bound(self.py(), &format!("{}.{}", module, qualname)) + } }; - name + #[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> { - #[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::().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::(); + 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::().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" + ); + }); + } } diff --git a/tests/test_methods.rs b/tests/test_methods.rs index 615e2dba0af..510ff5c8bb9 100644 --- a/tests/test_methods.rs +++ b/tests/test_methods.rs @@ -78,9 +78,8 @@ impl ClassMethod { } #[classmethod] - fn method_owned(cls: Py) -> PyResult { - let qualname = Python::with_gil(|gil| cls.bind(gil).qualname())?; - Ok(format!("{}.method_owned()!", qualname)) + fn method_owned(cls: Py, py: Python<'_>) -> PyResult { + Ok(format!("{}.method_owned()!", cls.bind(py).qualname()?)) } }