Skip to content

Commit

Permalink
Merge #2980
Browse files Browse the repository at this point in the history
2980: support `text_signature` on `#[new]` r=adamreichold a=davidhewitt

Closes #2866 

This is a breaking change for 0.19.0, because it starts autogenerating `text_signature` for `#[new]`. This could affect runtime behaviour if the user is relying on the class docs at runtime for some reason.

Guide & tests all updated accordingly.

`#[pyclass(text_signature = "...")]` is deprecated by this PR, however if it's set, it will be used in preference to `#[new]`.

(The signature / `text_signature` from `#[new]` will simply be ignored in this case. I figure that when users fix their deprecation warnings by removing `#[pyclass(text_signature = "...")]` then the `#[new]` signatures will start flowing properly, and this is good enough.)

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
  • Loading branch information
bors[bot] and davidhewitt authored Apr 20, 2023
2 parents 2f8bf51 + 8acb224 commit 78fe807
Show file tree
Hide file tree
Showing 20 changed files with 409 additions and 275 deletions.
27 changes: 17 additions & 10 deletions guide/src/class.md
Original file line number Diff line number Diff line change
Expand Up @@ -722,22 +722,20 @@ py_args=(), py_kwargs=None, name=World, num=-1, num_before=44

## Making class method signatures available to Python

The [`text_signature = "..."`](./function.md#text_signature) option for `#[pyfunction]` also works for classes and methods:
The [`text_signature = "..."`](./function.md#text_signature) option for `#[pyfunction]` also works for `#[pymethods]`:

```rust
# #![allow(dead_code)]
use pyo3::prelude::*;
use pyo3::types::PyType;

// it works even if the item is not documented:
#[pyclass(text_signature = "(c, d, /)")]
#[pyclass]
struct MyClass {}

#[pymethods]
impl MyClass {
// the signature for the constructor is attached
// to the struct definition instead.
#[new]
#[pyo3(text_signature = "(c, d)")]
fn new(c: i32, d: &str) -> Self {
Self {}
}
Expand All @@ -746,8 +744,9 @@ impl MyClass {
fn my_method(&self, e: i32, f: i32) -> i32 {
e + f
}
// similarly for classmethod arguments, use $cls
#[classmethod]
#[pyo3(text_signature = "(cls, e, f)")]
#[pyo3(text_signature = "($cls, e, f)")]
fn my_class_method(cls: &PyType, e: i32, f: i32) -> i32 {
e + f
}
Expand All @@ -773,7 +772,7 @@ impl MyClass {
# .call1((class,))?
# .call_method0("__str__")?
# .extract()?;
# assert_eq!(sig, "(c, d, /)");
# assert_eq!(sig, "(c, d)");
# } else {
# let doc: String = class.getattr("__doc__")?.extract()?;
# assert_eq!(doc, "");
Expand Down Expand Up @@ -802,7 +801,7 @@ impl MyClass {
# .call1((method,))?
# .call_method0("__str__")?
# .extract()?;
# assert_eq!(sig, "(cls, e, f)");
# assert_eq!(sig, "(e, f)"); // inspect.signature skips the $cls arg
# }
#
# {
Expand All @@ -822,7 +821,7 @@ impl MyClass {
# }
```

Note that `text_signature` on classes is not compatible with compilation in
Note that `text_signature` on `#[new]` is not compatible with compilation in
`abi3` mode until Python 3.10 or greater.

## #[pyclass] enums
Expand Down Expand Up @@ -1018,7 +1017,6 @@ impl pyo3::IntoPy<PyObject> for MyClass {
}

impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
const DOC: &'static str = "Class for demonstration\u{0}";
const IS_BASETYPE: bool = false;
const IS_SUBCLASS: bool = false;
type Layout = PyCell<MyClass>;
Expand All @@ -1041,6 +1039,15 @@ impl pyo3::impl_::pyclass::PyClassImpl for MyClass {
static TYPE_OBJECT: LazyTypeObject<MyClass> = LazyTypeObject::new();
&TYPE_OBJECT
}

fn doc(py: Python<'_>) -> pyo3::PyResult<&'static ::std::ffi::CStr> {
use pyo3::impl_::pyclass::*;
static DOC: pyo3::once_cell::GILOnceCell<::std::borrow::Cow<'static, ::std::ffi::CStr>> = pyo3::once_cell::GILOnceCell::new();
DOC.get_or_try_init(py, || {
let collector = PyClassImplCollector::<Self>::new();
build_pyclass_doc(<MyClass as pyo3::PyTypeInfo>::NAME, "", None.or_else(|| collector.new_text_signature()))
}).map(::std::ops::Deref::deref)
}
}

# Python::with_gil(|py| {
Expand Down
6 changes: 2 additions & 4 deletions guide/src/class/numeric.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ fn wrap(obj: &PyAny) -> Result<i32, PyErr> {
Ok(val as i32)
}
```
We also add documentation, via `///` comments and the `#[pyo3(text_signature = "...")]` attribute, both of which are visible to Python users.
We also add documentation, via `///` comments, which are visible to Python users.

```rust
# #![allow(dead_code)]
Expand All @@ -57,7 +57,6 @@ fn wrap(obj: &PyAny) -> Result<i32, PyErr> {
/// Did you ever hear the tragedy of Darth Signed The Overfloweth? I thought not.
/// It's not a story C would tell you. It's a Rust legend.
#[pyclass(module = "my_module")]
#[pyo3(text_signature = "(int)")]
struct Number(i32);

#[pymethods]
Expand Down Expand Up @@ -223,7 +222,6 @@ fn wrap(obj: &PyAny) -> Result<i32, PyErr> {
/// Did you ever hear the tragedy of Darth Signed The Overfloweth? I thought not.
/// It's not a story C would tell you. It's a Rust legend.
#[pyclass(module = "my_module")]
#[pyo3(text_signature = "(int)")]
struct Number(i32);

#[pymethods]
Expand Down Expand Up @@ -377,7 +375,7 @@ fn my_module(_py: Python<'_>, m: &PyModule) -> PyResult<()> {
# assert Number(12345234523452) == Number(1498514748)
# try:
# import inspect
# assert inspect.signature(Number).__str__() == '(int)'
# assert inspect.signature(Number).__str__() == '(value)'
# except ValueError:
# # Not supported with `abi3` before Python 3.10
# pass
Expand Down
4 changes: 1 addition & 3 deletions guide/src/function/signature.md
Original file line number Diff line number Diff line change
Expand Up @@ -272,9 +272,7 @@ impl MyClass {

The function signature is exposed to Python via the `__text_signature__` attribute. PyO3 automatically generates this for every `#[pyfunction]` and all `#[pymethods]` directly from the Rust function, taking into account any override done with the `#[pyo3(signature = (...))]` option.

This automatic generation has some limitations, which may be improved in the future:
- It will not include the value of default arguments, replacing them all with `...`. (`.pyi` type stub files commonly also use `...` for all default arguments in the same way.)
- Nothing is generated for the `#[new]` method of a `#[pyclass]`.
This automatic generation can only display the value of default arguments for strings, integers, boolean types, and `None`. Any other default arguments will be displayed as `...`. (`.pyi` type stub files commonly also use `...` for default arguments in the same way.)

In cases where the automatically-generated signature needs adjusting, it can [be overridden](#overriding-the-generated-signature) using the `#[pyo3(text_signature)]` option.)

Expand Down
1 change: 1 addition & 0 deletions newsfragments/2980.added.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Support `text_signature` option (and automatically generate signature) for `#[new]` in `#[pymethods]`.
1 change: 1 addition & 0 deletions newsfragments/2980.changed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Deprecate `text_signature` option on `#[pyclass]`.
2 changes: 2 additions & 0 deletions pyo3-macros-backend/src/deprecations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ use proc_macro2::{Span, TokenStream};
use quote::{quote_spanned, ToTokens};

pub enum Deprecation {
PyClassTextSignature,
PyFunctionArguments,
PyMethodArgsAttribute,
RequiredArgumentAfterOption,
Expand All @@ -10,6 +11,7 @@ pub enum Deprecation {
impl Deprecation {
fn ident(&self, span: Span) -> syn::Ident {
let string = match self {
Deprecation::PyClassTextSignature => "PYCLASS_TEXT_SIGNATURE",
Deprecation::PyFunctionArguments => "PYFUNCTION_ARGUMENTS",
Deprecation::PyMethodArgsAttribute => "PYMETHODS_ARGS_ATTRIBUTE",
Deprecation::RequiredArgumentAfterOption => "REQUIRED_ARGUMENT_AFTER_OPTION",
Expand Down
34 changes: 28 additions & 6 deletions pyo3-macros-backend/src/method.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Copyright (c) 2017-present PyO3 Project and Contributors

use crate::attributes::TextSignatureAttribute;
use crate::attributes::{TextSignatureAttribute, TextSignatureAttributeValue};
use crate::deprecations::{Deprecation, Deprecations};
use crate::params::impl_arg_params;
use crate::pyfunction::{DeprecatedArgs, FunctionSignature, PyFunctionArgPyO3Attributes};
Expand Down Expand Up @@ -593,6 +593,33 @@ impl<'a> FnSpec<'a> {
CallingConvention::TpNew => unreachable!("tp_new cannot get a methoddef"),
}
}

/// Forwards to [utils::get_doc] with the text signature of this spec.
pub fn get_doc(&self, attrs: &[syn::Attribute]) -> PythonDoc {
let text_signature = self
.text_signature_call_signature()
.map(|sig| format!("{}{}", self.python_name, sig));
utils::get_doc(attrs, text_signature)
}

/// Creates the parenthesised arguments list for `__text_signature__` snippet based on this spec's signature
/// and/or attributes. Prepend the callable name to make a complete `__text_signature__`.
pub fn text_signature_call_signature(&self) -> Option<String> {
let self_argument = match &self.tp {
// Getters / Setters / ClassAttribute are not callables on the Python side
FnType::Getter(_) | FnType::Setter(_) | FnType::ClassAttribute => return None,
FnType::Fn(_) => Some("self"),
FnType::FnModule => Some("module"),
FnType::FnClass => Some("cls"),
FnType::FnStatic | FnType::FnNew => None,
};

match self.text_signature.as_ref().map(|attr| &attr.value) {
Some(TextSignatureAttributeValue::Str(s)) => Some(s.value()),
None => Some(self.signature.text_signature(self_argument)),
Some(TextSignatureAttributeValue::Disabled(_)) => None,
}
}
}

#[derive(Debug)]
Expand Down Expand Up @@ -755,11 +782,6 @@ fn ensure_signatures_on_valid_method(
}
if let Some(text_signature) = text_signature {
match fn_type {
FnType::FnNew => bail_spanned!(
text_signature.kw.span() =>
"`text_signature` not allowed on `__new__`; if you want to add a signature on \
`__new__`, put it on the struct definition instead"
),
FnType::Getter(_) => {
bail_spanned!(text_signature.kw.span() => "`text_signature` not allowed with `getter`")
}
Expand Down
55 changes: 30 additions & 25 deletions pyo3-macros-backend/src/pyclass.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,11 @@ use std::borrow::Cow;
use crate::attributes::{
self, kw, take_pyo3_options, CrateAttribute, ExtendsAttribute, FreelistAttribute,
ModuleAttribute, NameAttribute, NameLitStr, TextSignatureAttribute,
TextSignatureAttributeValue,
};
use crate::deprecations::Deprecations;
use crate::deprecations::{Deprecation, Deprecations};
use crate::konst::{ConstAttributes, ConstSpec};
use crate::method::FnSpec;
use crate::pyfunction::text_signature_or_none;
use crate::pyimpl::{gen_py_const, PyClassMethodsType};
use crate::pymethod::{
impl_py_getter_def, impl_py_setter_def, MethodAndMethodDef, MethodAndSlotDef, PropertyType,
Expand Down Expand Up @@ -177,7 +177,11 @@ impl PyClassPyO3Options {
PyClassPyO3Option::Sequence(sequence) => set_option!(sequence),
PyClassPyO3Option::SetAll(set_all) => set_option!(set_all),
PyClassPyO3Option::Subclass(subclass) => set_option!(subclass),
PyClassPyO3Option::TextSignature(text_signature) => set_option!(text_signature),
PyClassPyO3Option::TextSignature(text_signature) => {
self.deprecations
.push(Deprecation::PyClassTextSignature, text_signature.span());
set_option!(text_signature)
}
PyClassPyO3Option::Unsendable(unsendable) => set_option!(unsendable),
PyClassPyO3Option::Weakref(weakref) => set_option!(weakref),
}
Expand All @@ -191,11 +195,7 @@ pub fn build_py_class(
methods_type: PyClassMethodsType,
) -> syn::Result<TokenStream> {
args.options.take_pyo3_options(&mut class.attrs)?;
let text_signature_string = text_signature_or_none(args.options.text_signature.as_ref());
let doc = utils::get_doc(
&class.attrs,
text_signature_string.map(|s| (get_class_python_name(&class.ident, &args), s)),
);
let doc = utils::get_doc(&class.attrs, None);
let krate = get_pyo3_crate(&args.options.krate);

if let Some(lt) = class.generics.lifetimes().next() {
Expand Down Expand Up @@ -452,12 +452,7 @@ pub fn build_py_enum(
bail_spanned!(enum_.brace_token.span => "#[pyclass] can't be used on enums without any variants");
}

let text_signature_string = text_signature_or_none(args.options.text_signature.as_ref());

let doc = utils::get_doc(
&enum_.attrs,
text_signature_string.map(|s| (get_class_python_name(&enum_.ident, &args), s)),
);
let doc = utils::get_doc(&enum_.attrs, None);
let enum_ = PyClassEnum::new(enum_)?;
impl_enum(enum_, &args, doc, method_type)
}
Expand Down Expand Up @@ -509,16 +504,6 @@ fn impl_enum(
methods_type: PyClassMethodsType,
) -> Result<TokenStream> {
let krate = get_pyo3_crate(&args.options.krate);
impl_enum_class(enum_, args, doc, methods_type, krate)
}

fn impl_enum_class(
enum_: PyClassEnum<'_>,
args: &PyClassArgs,
doc: PythonDoc,
methods_type: PyClassMethodsType,
krate: syn::Path,
) -> Result<TokenStream> {
let cls = enum_.ident;
let ty: syn::Type = syn::parse_quote!(#cls);
let variants = enum_.variants;
Expand Down Expand Up @@ -889,6 +874,18 @@ impl<'a> PyClassImplsBuilder<'a> {
fn impl_pyclassimpl(&self) -> Result<TokenStream> {
let cls = self.cls;
let doc = self.doc.as_ref().map_or(quote! {"\0"}, |doc| quote! {#doc});
let deprecated_text_signature = match self
.attr
.options
.text_signature
.as_ref()
.map(|attr| &attr.value)
{
Some(TextSignatureAttributeValue::Str(s)) => quote!(::std::option::Option::Some(#s)),
Some(TextSignatureAttributeValue::Disabled(_)) | None => {
quote!(::std::option::Option::None)
}
};
let is_basetype = self.attr.options.subclass.is_some();
let base = self
.attr
Expand Down Expand Up @@ -1009,7 +1006,6 @@ impl<'a> PyClassImplsBuilder<'a> {

Ok(quote! {
impl _pyo3::impl_::pyclass::PyClassImpl for #cls {
const DOC: &'static str = #doc;
const IS_BASETYPE: bool = #is_basetype;
const IS_SUBCLASS: bool = #is_subclass;
const IS_MAPPING: bool = #is_mapping;
Expand All @@ -1035,6 +1031,15 @@ impl<'a> PyClassImplsBuilder<'a> {
PyClassItemsIter::new(&INTRINSIC_ITEMS, #pymethods_items)
}

fn doc(py: _pyo3::Python<'_>) -> _pyo3::PyResult<&'static ::std::ffi::CStr> {
use _pyo3::impl_::pyclass::*;
static DOC: _pyo3::once_cell::GILOnceCell<::std::borrow::Cow<'static, ::std::ffi::CStr>> = _pyo3::once_cell::GILOnceCell::new();
DOC.get_or_try_init(py, || {
let collector = PyClassImplCollector::<Self>::new();
build_pyclass_doc(<#cls as _pyo3::PyTypeInfo>::NAME, #doc, #deprecated_text_signature.or_else(|| collector.new_text_signature()))
}).map(::std::ops::Deref::deref)
}

#dict_offset

#weaklist_offset
Expand Down
44 changes: 6 additions & 38 deletions pyo3-macros-backend/src/pyfunction.rs
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
// Copyright (c) 2017-present PyO3 Project and Contributors

use std::borrow::Cow;

use crate::{
attributes::{
self, get_pyo3_options, take_attributes, take_pyo3_options, CrateAttribute,
FromPyWithAttribute, NameAttribute, TextSignatureAttribute, TextSignatureAttributeValue,
FromPyWithAttribute, NameAttribute, TextSignatureAttribute,
},
deprecations::{Deprecation, Deprecations},
method::{self, CallingConvention, FnArg, FnType},
method::{self, CallingConvention, FnArg},
pymethod::check_generic,
utils::{self, ensure_not_async_fn, get_pyo3_crate},
utils::{ensure_not_async_fn, get_pyo3_crate},
};
use proc_macro2::TokenStream;
use quote::{format_ident, quote};
Expand Down Expand Up @@ -409,15 +407,6 @@ pub fn impl_wrap_pyfunction(

let ty = method::get_return_info(&func.sig.output);

let text_signature_string = text_signature_or_auto(text_signature.as_ref(), &signature, &tp);

let doc = utils::get_doc(
&func.attrs,
text_signature_string.map(|s| (Cow::Borrowed(&python_name), s)),
);

let krate = get_pyo3_crate(&krate);

let spec = method::FnSpec {
tp,
name: &func.sig.ident,
Expand All @@ -430,12 +419,14 @@ pub fn impl_wrap_pyfunction(
unsafety: func.sig.unsafety,
};

let krate = get_pyo3_crate(&krate);

let vis = &func.vis;
let name = &func.sig.ident;

let wrapper_ident = format_ident!("__pyfunction_{}", spec.name);
let wrapper = spec.get_wrapper_function(&wrapper_ident, None)?;
let methoddef = spec.get_methoddef(wrapper_ident, &doc);
let methoddef = spec.get_methoddef(wrapper_ident, &spec.get_doc(&func.attrs));

let wrapped_pyfunction = quote! {

Expand Down Expand Up @@ -480,26 +471,3 @@ fn type_is_pymodule(ty: &syn::Type) -> bool {
}
false
}

/// Helper to get a text signature string, or None if unset or disabled
pub(crate) fn text_signature_or_none(
text_signature: Option<&TextSignatureAttribute>,
) -> Option<String> {
match text_signature.map(|attr| &attr.value) {
Some(TextSignatureAttributeValue::Str(s)) => Some(s.value()),
Some(TextSignatureAttributeValue::Disabled(_)) | None => None,
}
}

/// Helper to get a text signature string, using automatic generation if unset, or None if disabled
pub(crate) fn text_signature_or_auto(
text_signature: Option<&TextSignatureAttribute>,
signature: &FunctionSignature<'_>,
fn_type: &FnType,
) -> Option<String> {
match text_signature.map(|attr| &attr.value) {
Some(TextSignatureAttributeValue::Str(s)) => Some(s.value()),
None => Some(signature.text_signature(fn_type)),
Some(TextSignatureAttributeValue::Disabled(_)) => None,
}
}
Loading

0 comments on commit 78fe807

Please sign in to comment.