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 May 20, 2024
1 parent 3e4b3c5 commit 1ce08ad
Show file tree
Hide file tree
Showing 2 changed files with 142 additions and 45 deletions.
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
179 changes: 134 additions & 45 deletions src/types/typeobject.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,9 +4,7 @@ use crate::types::any::PyAnyMethods;
#[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 @@ -70,15 +68,17 @@ impl PyType {
Self::from_borrowed_type_ptr(py, p).into_gil_ref()
}

/// Gets the short name of the `PyType`.
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`.
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 @@ -109,12 +109,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 short name of the `PyType`.
fn name(&self) -> PyResult<String>;

/// Gets the [qualified name](https://docs.python.org/3/glossary.html#term-qualified-name) of the `PyType`.
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://docs.python.org/3/glossary.html#term-qualified-name) 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 All @@ -137,10 +143,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 @@ -158,6 +177,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 All @@ -179,44 +245,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::any::PyAnyMethods;
use crate::types::typeobject::PyTypeMethods;
use crate::types::{PyBool, PyLong};
use crate::types::{PyBool, PyDelta, PyLong, PyModule, PyType};
use crate::Python;

#[test]
Expand All @@ -237,4 +270,60 @@ mod tests {
.unwrap());
});
}

#[test]
fn test_type_names_standard() {
Python::with_gil(|py| {
let bool_type = py.get_type_bound::<PyDelta>();
assert_eq!(bool_type.name().unwrap(), "timedelta");
assert_eq!(bool_type.qualname().unwrap(), "timedelta");
assert_eq!(bool_type.module().unwrap(), "datetime");
assert_eq!(
bool_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 1ce08ad

Please sign in to comment.