removes implicit default of trailing optional arguments (see #2935)

Author: Icxolu

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
-
-<div class="warning">
-
-⚠️ 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.
-</div>
-
-
-As a convenience, functions without a `#[pyo3(signature = (...))]` option will treat trailing `Option<T>` 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>) -> 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<T>` 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>) -> 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<T>` argument is surrounded by arguments which aren't `Option<T>`.
-
 ## 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.

guide/src/migration.md

@@ -872,7 +872,7 @@ Python::with_gil(|py| -> PyResult<()> {
 <details>
 <summary><small>Click to expand</small></summary>
 
-[Trailing `Option<T>` 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<T>` argument without using `#[pyo3(signature = (...))]` to specify the intended defaults. In PyO3 0.20, this becomes a hard error.
+Trailing `Option<T>` 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<T>` 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::*;
 

newsfragments/4729.removed.md

@@ -0,0 +1 @@
+removes implicit default of trailing optional arguments (see #2935)

pyo3-macros-backend/src/deprecations.rs

@@ -9,7 +9,6 @@
 mod utils;
 
 mod attributes;
-mod deprecations;
 mod frompyobject;
 mod intopyobject;
 mod konst;

pyo3-macros-backend/src/lib.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

pyo3-macros-backend/src/method.rs

@@ -245,10 +245,7 @@ pub(crate) fn impl_regular_arg_param(
     // Option<T> 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());

pyo3-macros-backend/src/params.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<MethodAndMethodDef> {
-    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);
 

pyo3-macros-backend/src/pyclass.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 {

pyo3-macros-backend/src/pyfunction.rs

@@ -459,25 +459,19 @@ impl<'a> FunctionSignature<'a> {
     }
 
     /// Without `#[pyo3(signature)]` or `#[args]` - just take the Rust function arguments as positional.
-    pub fn from_arguments(arguments: Vec<FnArg<'a>>) -> syn::Result<Self> {
+    pub fn from_arguments(arguments: Vec<FnArg<'a>>) -> Self {
         let mut python_signature = PythonSignature::default();
         for arg in &arguments {
             // Python<'_> arguments don't show in Python signature
             if matches!(arg, FnArg::Py(..) | FnArg::CancelHandle(..)) {
                 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 {