From 6abb69d9fa890b61a8048dd5be5084a4640f7621 Mon Sep 17 00:00:00 2001
From: Icxolu <10486322+Icxolu@users.noreply.github.com>
Date: Thu, 28 Nov 2024 23:38:11 +0100
Subject: [PATCH] removes implicit default of trailing optional arguments (see
#2935) (#4729)
---
guide/src/function/signature.md | 78 +------------------
guide/src/migration.md | 4 +-
newsfragments/4729.removed.md | 1 +
pyo3-macros-backend/src/deprecations.rs | 54 -------------
pyo3-macros-backend/src/lib.rs | 1 -
pyo3-macros-backend/src/method.rs | 9 +--
pyo3-macros-backend/src/params.rs | 52 ++++++-------
pyo3-macros-backend/src/pyclass.rs | 6 +-
pyo3-macros-backend/src/pyfunction.rs | 2 +-
.../src/pyfunction/signature.rs | 20 ++---
pyo3-macros-backend/src/pymethod.rs | 3 -
tests/test_compile_error.rs | 1 -
tests/ui/deprecations.rs | 28 -------
tests/ui/deprecations.stderr | 29 -------
tests/ui/invalid_pyfunctions.rs | 3 -
tests/ui/invalid_pyfunctions.stderr | 23 ++----
16 files changed, 49 insertions(+), 265 deletions(-)
create mode 100644 newsfragments/4729.removed.md
delete mode 100644 pyo3-macros-backend/src/deprecations.rs
delete mode 100644 tests/ui/deprecations.rs
delete mode 100644 tests/ui/deprecations.stderr
diff --git a/guide/src/function/signature.md b/guide/src/function/signature.md
index 8ebe74456a1..431cad87bfd 100644
--- a/guide/src/function/signature.md
+++ b/guide/src/function/signature.md
@@ -2,7 +2,7 @@
The `#[pyfunction]` attribute also accepts parameters to control how the generated Python function accepts arguments. Just like in Python, arguments can be positional-only, keyword-only, or accept either. `*args` lists and `**kwargs` dicts can also be accepted. These parameters also work for `#[pymethods]` which will be introduced in the [Python Classes](../class.md) section of the guide.
-Like Python, by default PyO3 accepts all arguments as either positional or keyword arguments. Most arguments are required by default, except for trailing `Option<_>` arguments, which are [implicitly given a default of `None`](#trailing-optional-arguments). This behaviour can be configured by the `#[pyo3(signature = (...))]` option which allows writing a signature in Python syntax.
+Like Python, by default PyO3 accepts all arguments as either positional or keyword arguments. All arguments are required by default. This behaviour can be configured by the `#[pyo3(signature = (...))]` option which allows writing a signature in Python syntax.
This section of the guide goes into detail about use of the `#[pyo3(signature = (...))]` option and its related option `#[pyo3(text_signature = "...")]`
@@ -118,82 +118,6 @@ num=-1
> }
> ```
-## Trailing optional arguments
-
-
-
-⚠️ Warning: This behaviour is being phased out 🛠️
-
-The special casing of trailing optional arguments is deprecated. In a future `pyo3` version, arguments of type `Option<..>` will share the same behaviour as other arguments, they are required unless a default is set using `#[pyo3(signature = (...))]`.
-
-This is done to better align the Python and Rust definition of such functions and make it more intuitive to rewrite them from Python in Rust. Specifically `def some_fn(a: int, b: Optional[int]): ...` will not automatically default `b` to `none`, but requires an explicit default if desired, where as in current `pyo3` it is handled the other way around.
-
-During the migration window a `#[pyo3(signature = (...))]` will be required to silence the deprecation warning. After support for trailing optional arguments is fully removed, the signature attribute can be removed if all arguments should be required.
-
-
-
-As a convenience, functions without a `#[pyo3(signature = (...))]` option will treat trailing `Option` arguments as having a default of `None`. In the example below, PyO3 will create `increment` with a signature of `increment(x, amount=None)`.
-
-```rust
-#![allow(deprecated)]
-use pyo3::prelude::*;
-
-/// Returns a copy of `x` increased by `amount`.
-///
-/// If `amount` is unspecified or `None`, equivalent to `x + 1`.
-#[pyfunction]
-fn increment(x: u64, amount: Option) -> u64 {
- x + amount.unwrap_or(1)
-}
-#
-# fn main() -> PyResult<()> {
-# Python::with_gil(|py| {
-# let fun = pyo3::wrap_pyfunction!(increment, py)?;
-#
-# let inspect = PyModule::import(py, "inspect")?.getattr("signature")?;
-# let sig: String = inspect
-# .call1((fun,))?
-# .call_method0("__str__")?
-# .extract()?;
-#
-# #[cfg(Py_3_8)] // on 3.7 the signature doesn't render b, upstream bug?
-# assert_eq!(sig, "(x, amount=None)");
-#
-# Ok(())
-# })
-# }
-```
-
-To make trailing `Option` arguments required, but still accept `None`, add a `#[pyo3(signature = (...))]` annotation. For the example above, this would be `#[pyo3(signature = (x, amount))]`:
-
-```rust
-# use pyo3::prelude::*;
-#[pyfunction]
-#[pyo3(signature = (x, amount))]
-fn increment(x: u64, amount: Option) -> u64 {
- x + amount.unwrap_or(1)
-}
-#
-# fn main() -> PyResult<()> {
-# Python::with_gil(|py| {
-# let fun = pyo3::wrap_pyfunction!(increment, py)?;
-#
-# let inspect = PyModule::import(py, "inspect")?.getattr("signature")?;
-# let sig: String = inspect
-# .call1((fun,))?
-# .call_method0("__str__")?
-# .extract()?;
-#
-# #[cfg(Py_3_8)] // on 3.7 the signature doesn't render b, upstream bug?
-# assert_eq!(sig, "(x, amount)");
-#
-# Ok(())
-# })
-# }
-```
-
-To help avoid confusion, PyO3 requires `#[pyo3(signature = (...))]` when an `Option` argument is surrounded by arguments which aren't `Option`.
-
## Making the function signature available to Python
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.
diff --git a/guide/src/migration.md b/guide/src/migration.md
index 8a8f3694f6d..35126dfcaef 100644
--- a/guide/src/migration.md
+++ b/guide/src/migration.md
@@ -872,7 +872,7 @@ Python::with_gil(|py| -> PyResult<()> {
Click to expand
-[Trailing `Option` arguments](./function/signature.md#trailing-optional-arguments) have an automatic default of `None`. To avoid unwanted changes when modifying function signatures, in PyO3 0.18 it was deprecated to have a required argument after an `Option` argument without using `#[pyo3(signature = (...))]` to specify the intended defaults. In PyO3 0.20, this becomes a hard error.
+Trailing `Option` arguments have an automatic default of `None`. To avoid unwanted changes when modifying function signatures, in PyO3 0.18 it was deprecated to have a required argument after an `Option` argument without using `#[pyo3(signature = (...))]` to specify the intended defaults. In PyO3 0.20, this becomes a hard error.
Before:
@@ -1095,7 +1095,7 @@ Starting with PyO3 0.18, this is deprecated and a future PyO3 version will requi
Before, x in the below example would be required to be passed from Python code:
-```rust,compile_fail
+```rust,compile_fail,ignore
# #![allow(dead_code)]
# use pyo3::prelude::*;
diff --git a/newsfragments/4729.removed.md b/newsfragments/4729.removed.md
new file mode 100644
index 00000000000..da1498ee69f
--- /dev/null
+++ b/newsfragments/4729.removed.md
@@ -0,0 +1 @@
+removes implicit default of trailing optional arguments (see #2935)
\ No newline at end of file
diff --git a/pyo3-macros-backend/src/deprecations.rs b/pyo3-macros-backend/src/deprecations.rs
deleted file mode 100644
index df48c9da417..00000000000
--- a/pyo3-macros-backend/src/deprecations.rs
+++ /dev/null
@@ -1,54 +0,0 @@
-use crate::method::{FnArg, FnSpec};
-use proc_macro2::TokenStream;
-use quote::quote_spanned;
-
-pub(crate) fn deprecate_trailing_option_default(spec: &FnSpec<'_>) -> TokenStream {
- if spec.signature.attribute.is_none()
- && spec.tp.signature_attribute_allowed()
- && spec.signature.arguments.iter().any(|arg| {
- if let FnArg::Regular(arg) = arg {
- arg.option_wrapped_type.is_some()
- } else {
- false
- }
- })
- {
- use std::fmt::Write;
- let mut deprecation_msg = String::from(
- "this function has implicit defaults for the trailing `Option` arguments \n\
- = note: these implicit defaults are being phased out \n\
- = help: add `#[pyo3(signature = (",
- );
- spec.signature.arguments.iter().for_each(|arg| {
- match arg {
- FnArg::Regular(arg) => {
- if arg.option_wrapped_type.is_some() {
- write!(deprecation_msg, "{}=None, ", arg.name)
- } else {
- write!(deprecation_msg, "{}, ", arg.name)
- }
- }
- FnArg::VarArgs(arg) => write!(deprecation_msg, "{}, ", arg.name),
- FnArg::KwArgs(arg) => write!(deprecation_msg, "{}, ", arg.name),
- FnArg::Py(_) | FnArg::CancelHandle(_) => Ok(()),
- }
- .expect("writing to `String` should not fail");
- });
-
- //remove trailing space and comma
- deprecation_msg.pop();
- deprecation_msg.pop();
-
- deprecation_msg.push_str(
- "))]` to this function to silence this warning and keep the current behavior",
- );
- quote_spanned! { spec.name.span() =>
- #[deprecated(note = #deprecation_msg)]
- #[allow(dead_code)]
- const SIGNATURE: () = ();
- const _: () = SIGNATURE;
- }
- } else {
- TokenStream::new()
- }
-}
diff --git a/pyo3-macros-backend/src/lib.rs b/pyo3-macros-backend/src/lib.rs
index d6c8f287332..7893a94af98 100644
--- a/pyo3-macros-backend/src/lib.rs
+++ b/pyo3-macros-backend/src/lib.rs
@@ -9,7 +9,6 @@
mod utils;
mod attributes;
-mod deprecations;
mod frompyobject;
mod intopyobject;
mod konst;
diff --git a/pyo3-macros-backend/src/method.rs b/pyo3-macros-backend/src/method.rs
index f99e64562b7..a1d7a95df35 100644
--- a/pyo3-macros-backend/src/method.rs
+++ b/pyo3-macros-backend/src/method.rs
@@ -6,7 +6,6 @@ use proc_macro2::{Span, TokenStream};
use quote::{format_ident, quote, quote_spanned, ToTokens};
use syn::{ext::IdentExt, spanned::Spanned, Ident, Result};
-use crate::deprecations::deprecate_trailing_option_default;
use crate::pyversions::is_abi3_before;
use crate::utils::{Ctx, LitCStr};
use crate::{
@@ -474,7 +473,7 @@ impl<'a> FnSpec<'a> {
let signature = if let Some(signature) = signature {
FunctionSignature::from_arguments_and_attribute(arguments, signature)?
} else {
- FunctionSignature::from_arguments(arguments)?
+ FunctionSignature::from_arguments(arguments)
};
let convention = if matches!(fn_type, FnType::FnNew | FnType::FnNewClass(_)) {
@@ -745,8 +744,6 @@ impl<'a> FnSpec<'a> {
quote!(#func_name)
};
- let deprecation = deprecate_trailing_option_default(self);
-
Ok(match self.convention {
CallingConvention::Noargs => {
let mut holders = Holders::new();
@@ -767,7 +764,6 @@ impl<'a> FnSpec<'a> {
py: #pyo3_path::Python<'py>,
_slf: *mut #pyo3_path::ffi::PyObject,
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
- #deprecation
let function = #rust_name; // Shadow the function name to avoid #3017
#init_holders
let result = #call;
@@ -789,7 +785,6 @@ impl<'a> FnSpec<'a> {
_nargs: #pyo3_path::ffi::Py_ssize_t,
_kwnames: *mut #pyo3_path::ffi::PyObject
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
- #deprecation
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#init_holders
@@ -811,7 +806,6 @@ impl<'a> FnSpec<'a> {
_args: *mut #pyo3_path::ffi::PyObject,
_kwargs: *mut #pyo3_path::ffi::PyObject
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
- #deprecation
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#init_holders
@@ -836,7 +830,6 @@ impl<'a> FnSpec<'a> {
_kwargs: *mut #pyo3_path::ffi::PyObject
) -> #pyo3_path::PyResult<*mut #pyo3_path::ffi::PyObject> {
use #pyo3_path::impl_::callback::IntoPyCallbackOutput;
- #deprecation
let function = #rust_name; // Shadow the function name to avoid #3017
#arg_convert
#init_holders
diff --git a/pyo3-macros-backend/src/params.rs b/pyo3-macros-backend/src/params.rs
index 67054458c98..9517d35b25c 100644
--- a/pyo3-macros-backend/src/params.rs
+++ b/pyo3-macros-backend/src/params.rs
@@ -245,10 +245,7 @@ pub(crate) fn impl_regular_arg_param(
// Option arguments have special treatment: the default should be specified _without_ the
// Some() wrapper. Maybe this should be changed in future?!
if arg.option_wrapped_type.is_some() {
- default = Some(default.map_or_else(
- || quote!(::std::option::Option::None),
- |tokens| some_wrap(tokens, ctx),
- ));
+ default = default.map(|tokens| some_wrap(tokens, ctx));
}
if arg.from_py_with.is_some() {
@@ -273,31 +270,32 @@ pub(crate) fn impl_regular_arg_param(
)?
}
}
- } else if arg.option_wrapped_type.is_some() {
- let holder = holders.push_holder(arg.name.span());
- quote_arg_span! {
- #pyo3_path::impl_::extract_argument::extract_optional_argument(
- #arg_value,
- &mut #holder,
- #name_str,
- #[allow(clippy::redundant_closure)]
- {
- || #default
- }
- )?
- }
} else if let Some(default) = default {
let holder = holders.push_holder(arg.name.span());
- quote_arg_span! {
- #pyo3_path::impl_::extract_argument::extract_argument_with_default(
- #arg_value,
- &mut #holder,
- #name_str,
- #[allow(clippy::redundant_closure)]
- {
- || #default
- }
- )?
+ if arg.option_wrapped_type.is_some() {
+ quote_arg_span! {
+ #pyo3_path::impl_::extract_argument::extract_optional_argument(
+ #arg_value,
+ &mut #holder,
+ #name_str,
+ #[allow(clippy::redundant_closure)]
+ {
+ || #default
+ }
+ )?
+ }
+ } else {
+ quote_arg_span! {
+ #pyo3_path::impl_::extract_argument::extract_argument_with_default(
+ #arg_value,
+ &mut #holder,
+ #name_str,
+ #[allow(clippy::redundant_closure)]
+ {
+ || #default
+ }
+ )?
+ }
}
} else {
let holder = holders.push_holder(arg.name.span());
diff --git a/pyo3-macros-backend/src/pyclass.rs b/pyo3-macros-backend/src/pyclass.rs
index 16e3c58f6ad..3ac3fa358db 100644
--- a/pyo3-macros-backend/src/pyclass.rs
+++ b/pyo3-macros-backend/src/pyclass.rs
@@ -1660,7 +1660,7 @@ fn complex_enum_struct_variant_new<'a>(
constructor.into_signature(),
)?
} else {
- crate::pyfunction::FunctionSignature::from_arguments(args)?
+ crate::pyfunction::FunctionSignature::from_arguments(args)
};
let spec = FnSpec {
@@ -1714,7 +1714,7 @@ fn complex_enum_tuple_variant_new<'a>(
constructor.into_signature(),
)?
} else {
- crate::pyfunction::FunctionSignature::from_arguments(args)?
+ crate::pyfunction::FunctionSignature::from_arguments(args)
};
let spec = FnSpec {
@@ -1737,7 +1737,7 @@ fn complex_enum_variant_field_getter<'a>(
field_span: Span,
ctx: &Ctx,
) -> Result {
- let signature = crate::pyfunction::FunctionSignature::from_arguments(vec![])?;
+ let signature = crate::pyfunction::FunctionSignature::from_arguments(vec![]);
let self_type = crate::method::SelfType::TryFromBoundRef(field_span);
diff --git a/pyo3-macros-backend/src/pyfunction.rs b/pyo3-macros-backend/src/pyfunction.rs
index 3059025caf7..f28fa795177 100644
--- a/pyo3-macros-backend/src/pyfunction.rs
+++ b/pyo3-macros-backend/src/pyfunction.rs
@@ -239,7 +239,7 @@ pub fn impl_wrap_pyfunction(
let signature = if let Some(signature) = signature {
FunctionSignature::from_arguments_and_attribute(arguments, signature)?
} else {
- FunctionSignature::from_arguments(arguments)?
+ FunctionSignature::from_arguments(arguments)
};
let spec = method::FnSpec {
diff --git a/pyo3-macros-backend/src/pyfunction/signature.rs b/pyo3-macros-backend/src/pyfunction/signature.rs
index 0a2d861d2b1..deea3dfa052 100644
--- a/pyo3-macros-backend/src/pyfunction/signature.rs
+++ b/pyo3-macros-backend/src/pyfunction/signature.rs
@@ -459,7 +459,7 @@ impl<'a> FunctionSignature<'a> {
}
/// Without `#[pyo3(signature)]` or `#[args]` - just take the Rust function arguments as positional.
- pub fn from_arguments(arguments: Vec>) -> syn::Result {
+ pub fn from_arguments(arguments: Vec>) -> Self {
let mut python_signature = PythonSignature::default();
for arg in &arguments {
// Python<'_> arguments don't show in Python signature
@@ -467,17 +467,11 @@ impl<'a> FunctionSignature<'a> {
continue;
}
- if let FnArg::Regular(RegularArg {
- ty,
- option_wrapped_type: None,
- ..
- }) = arg
- {
+ if let FnArg::Regular(RegularArg { .. }) = arg {
// This argument is required, all previous arguments must also have been required
- ensure_spanned!(
- python_signature.required_positional_parameters == python_signature.positional_parameters.len(),
- ty.span() => "required arguments after an `Option<_>` argument are ambiguous\n\
- = help: add a `#[pyo3(signature)]` annotation on this function to unambiguously specify the default values for all optional parameters"
+ assert_eq!(
+ python_signature.required_positional_parameters,
+ python_signature.positional_parameters.len(),
);
python_signature.required_positional_parameters =
@@ -489,11 +483,11 @@ impl<'a> FunctionSignature<'a> {
.push(arg.name().unraw().to_string());
}
- Ok(Self {
+ Self {
arguments,
python_signature,
attribute: None,
- })
+ }
}
fn default_value_for_parameter(&self, parameter: &str) -> String {
diff --git a/pyo3-macros-backend/src/pymethod.rs b/pyo3-macros-backend/src/pymethod.rs
index 1254a8d510b..3d2975e4885 100644
--- a/pyo3-macros-backend/src/pymethod.rs
+++ b/pyo3-macros-backend/src/pymethod.rs
@@ -2,7 +2,6 @@ use std::borrow::Cow;
use std::ffi::CString;
use crate::attributes::{NameAttribute, RenamingRule};
-use crate::deprecations::deprecate_trailing_option_default;
use crate::method::{CallingConvention, ExtractErrorMode, PyArg};
use crate::params::{impl_regular_arg_param, Holders};
use crate::utils::PythonDoc;
@@ -685,9 +684,7 @@ pub fn impl_py_setter_def(
ctx,
);
- let deprecation = deprecate_trailing_option_default(spec);
quote! {
- #deprecation
#from_py_with
let _val = #extract;
}
diff --git a/tests/test_compile_error.rs b/tests/test_compile_error.rs
index e4e80e90263..b165c911735 100644
--- a/tests/test_compile_error.rs
+++ b/tests/test_compile_error.rs
@@ -21,7 +21,6 @@ fn test_compile_errors() {
t.compile_fail("tests/ui/invalid_pymethod_names.rs");
t.compile_fail("tests/ui/invalid_pymodule_args.rs");
t.compile_fail("tests/ui/reject_generics.rs");
- t.compile_fail("tests/ui/deprecations.rs");
t.compile_fail("tests/ui/invalid_closure.rs");
t.compile_fail("tests/ui/pyclass_send.rs");
t.compile_fail("tests/ui/invalid_argument_attributes.rs");
diff --git a/tests/ui/deprecations.rs b/tests/ui/deprecations.rs
deleted file mode 100644
index 47b00d7eeee..00000000000
--- a/tests/ui/deprecations.rs
+++ /dev/null
@@ -1,28 +0,0 @@
-#![deny(deprecated)]
-#![allow(dead_code)]
-
-use pyo3::prelude::*;
-
-fn extract_options(obj: &Bound<'_, PyAny>) -> PyResult