Skip to content

Commit

Permalink
Revamp PyType name functions to match PEP 737
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
aneeshusa committed Jun 4, 2024
1 parent 93ef056 commit f394a71
Show file tree
Hide file tree
Showing 6 changed files with 208 additions and 50 deletions.
55 changes: 55 additions & 0 deletions guide/src/migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,61 @@ enum SimpleEnum {
```
</details>

### `PyType::name` now returns `PyResult<String>` instead of `PyResult<Cow<'_, str>>`
<details open>
<summary><small>Click to expand</small></summary>

This function previously would try to reuse a raw C API field (`tp_name`) in which case it would return a `Cow::Borrowed`,
but this caused inconsistency with Python.
It now always returned a fully owned String.

To migrate, remove any `.into_owned()` and `.into_mut()` calls on the output of `PyType::name`.

Before:

```rust
# #![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);

let mut name_upper = bool_type.name()?;
name_upper.make_ascii_uppercase();
println!("Hello, {}", name_upper);

Ok(())
})
# }
```
</details>



## from 0.20.* to 0.21
<details open>
<summary><small>Click to expand</small></summary>
Expand Down
4 changes: 4 additions & 0 deletions newsfragments/4196.added.md
Original file line number Diff line number Diff line change
@@ -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.
1 change: 1 addition & 0 deletions newsfragments/4196.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change `PyType::name` to always match Python `__name__`.
8 changes: 8 additions & 0 deletions pyo3-ffi/src/object.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
7 changes: 3 additions & 4 deletions pytests/src/misc.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
use pyo3::{prelude::*, types::PyDict};
use std::borrow::Cow;

#[pyfunction]
fn issue_219() {
Expand All @@ -8,8 +7,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_full_qualified_name(obj: &Bound<'_, PyAny>) -> PyResult<String> {
obj.get_type().fully_qualified_name()
}

#[pyfunction]
Expand All @@ -33,7 +32,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_full_qualified_name, m)?)?;
m.add_function(wrap_pyfunction!(accepts_bool, m)?)?;
m.add_function(wrap_pyfunction!(get_item_and_run_callback, m)?)?;
Ok(())
Expand Down
183 changes: 137 additions & 46 deletions src/types/typeobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,6 @@ 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;
/// Represents a reference to a Python `type object`.
#[repr(transparent)]
pub struct PyType(PyAny);
Expand Down Expand Up @@ -71,15 +68,18 @@ impl PyType {
Self::from_borrowed_type_ptr(py, p).into_gil_ref()
}

/// Gets the name of the `PyType`. Equivalent to `self.__name__` in Python.
pub fn name(&self) -> PyResult<String> {
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<String> {
self.as_borrowed().qualname()
}

/// Gets the full name, which includes the module, of the `PyType`.
pub fn name(&self) -> PyResult<Cow<'_, str>> {
self.as_borrowed().name()
}
// `module` and `fully_qualified_name` intentionally omitted

/// Checks whether `self` is a subclass of `other`.
///
Expand Down Expand Up @@ -110,12 +110,19 @@ 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<String>;

/// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
/// Equivalent to `self.__qualname__` in Python.
fn qualname(&self) -> PyResult<String>;

/// Gets the name of the module defining the `PyType`.
fn module(&self) -> PyResult<String>;

/// Gets the [fully qualified name](https://peps.python.org/pep-0737/#add-pytype-getfullyqualifiedname-function) of the `PyType`.
fn fully_qualified_name(&self) -> PyResult<String>;

/// Checks whether `self` is a subclass of `other`.
///
/// Equivalent to the Python expression `issubclass(self, other)`.
Expand Down Expand Up @@ -148,10 +155,23 @@ 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<String> {
#[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))]
let name = self.getattr(intern!(self.py(), "__name__"))?.extract();

#[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_11))))]
let name = {
use crate::ffi_ptr_ext::FfiPtrExt;
let obj =
unsafe { ffi::PyType_GetName(self.as_type_ptr()).assume_owned_or_err(self.py())? };

obj.extract()
};

name
}

/// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
fn qualname(&self) -> PyResult<String> {
#[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_11)))]
let name = self.getattr(intern!(self.py(), "__qualname__"))?.extract();
Expand All @@ -169,6 +189,53 @@ impl<'py> PyTypeMethods<'py> for Bound<'py, PyType> {
name
}

/// Gets the name of the module defining the `PyType`.
fn module(&self) -> PyResult<String> {
#[cfg(any(Py_LIMITED_API, PyPy, not(Py_3_13)))]
let name = self.getattr(intern!(self.py(), "__module__"))?.extract();

#[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_13))))]
let name = {
use crate::ffi_ptr_ext::FfiPtrExt;
let obj = unsafe {
ffi::PyType_GetModuleName(self.as_type_ptr()).assume_owned_or_err(self.py())?
};

obj.extract()
};

name
}

/// Gets the [fully qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
fn fully_qualified_name(&self) -> PyResult<String> {
#[cfg(any(Py_LIMITED_API, PyPy, 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::<&str>()?;
if module_str == "builtins" || module_str == "__main__" {
qualname.extract()
} else {
Ok(format!("{}.{}", module, qualname))
}
};

#[cfg(not(any(Py_LIMITED_API, PyPy, not(Py_3_13))))]
let name = {
use crate::ffi_ptr_ext::FfiPtrExt;
let obj = unsafe {
ffi::PyType_GetFullyQualifiedName(self.as_type_ptr())
.assume_owned_or_err(self.py())?
};

obj.extract()
};

name
}

/// Checks whether `self` is a subclass of `other`.
///
/// Equivalent to the Python expression `issubclass(self, other)`.
Expand Down Expand Up @@ -232,43 +299,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, PyDelta, PyInt, PyLong, PyModule, PyTuple, PyType, PyTypeMethods,
};
use crate::PyAny;
use crate::Python;

Expand Down Expand Up @@ -330,4 +365,60 @@ mod tests {
.unwrap());
});
}

#[test]
fn test_type_names_standard() {
Python::with_gil(|py| {
let timedelta_type = py.get_type_bound::<PyDelta>();
assert_eq!(timedelta_type.name().unwrap(), "timedelta");
assert_eq!(timedelta_type.qualname().unwrap(), "timedelta");
assert_eq!(timedelta_type.module().unwrap(), "datetime");
assert_eq!(
timedelta_type.fully_qualified_name().unwrap(),
"datetime.timedelta"
);
});
}

#[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"
);
});
}
}

0 comments on commit f394a71

Please sign in to comment.