Merge #2784 #2813

2784: Automatically generate `__text_signature__` for all functions r=davidhewitt a=davidhewitt

This PR makes it so that PyO3 generates `__text_signature__` by default for all functions. It also introduces `#[pyo3(text_signature = false)]` to disable the built-in generation.

There are a few limitations which we can improve later:
 - All default values are currently set to `...`. I think this is ok because `.pyi` files often do the same. Maybe for numbers, strings, `None` and `True`/`False` we could render these in a future PR.
 - No support for `#[new]` yet.

Alternative design ideas:
- Only autogenerate for methods with `#[pyo3(signature = (...))]` annotation. I started with this, and then decided it made sense to do it for everything.
- Opt-out with `#[pyo3(text_signature = None)]`. This is slightly harder to parse in the macro, but matches the final result in Python better, so if this looks preferable to others, I can change from `text_signature = false` to `text_signature = None`.

There's some small tidying up / refactoring to do before this merges (happy to take suggestions on this), however the general logic, design and docs are ready for review.


2813: ci: run pyo3-ffi-check using nox r=davidhewitt a=davidhewitt

Makes it easier to run pyo3-ffi-check locally :)

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>

.github/workflows/build.yml

@@ -58,9 +58,11 @@ jobs:
           set -x
           cargo update -p indexmap --precise 1.6.2
           cargo update -p hashbrown:0.12.3 --precise 0.9.1
+          # string_cache 0.8.4 depends on parking_lot 0.12
+          cargo update -p string_cache:0.8.4 --precise 0.8.3
           PROJECTS=("." "examples/decorator" "examples/maturin-starter" "examples/setuptools-rust-starter" "examples/word-count")
           for PROJ in ${PROJECTS[@]}; do
-            cargo update --manifest-path "$PROJ/Cargo.toml" -p parking_lot --precise 0.11.0
+            cargo update --manifest-path "$PROJ/Cargo.toml" -p parking_lot:0.12.1 --precise 0.11.2
             cargo update --manifest-path "$PROJ/Cargo.toml" -p once_cell --precise 1.14.0
           done
           cargo update --manifest-path "examples/word-count/Cargo.toml" -p rayon --precise 1.5.3
@@ -131,7 +133,7 @@ jobs:
           nox -s test-py
         env:
           CARGO_TARGET_DIR: ${{ github.workspace }}/target
-      
+
       - uses: dorny/paths-filter@v2
         # pypy 3.7 and 3.8 are not PEP 3123 compliant so fail checks here
         if: ${{ inputs.rust == 'stable' && inputs.python-version != 'pypy-3.7' && inputs.python-version != 'pypy-3.8' }}
@@ -143,18 +145,14 @@ jobs:
               - 'pyo3-ffi-check/**'
               - '.github/workflows/ci.yml'
               - '.github/workflows/build.yml'
-      
-      - run: cargo doc -p pyo3-ffi
-        working-directory: pyo3-ffi-check
-        if: ${{ steps.ffi-changes.outputs.changed == 'true' && inputs.rust == 'stable' && inputs.python-version != 'pypy-3.7' && inputs.python-version != 'pypy-3.8' }}
 
       - name: Run pyo3-ffi-check
-        run: cargo run
-        working-directory: pyo3-ffi-check
+        run: nox -s ffi-check
         # Allow failure on PyPy for now
         continue-on-error: ${{ startsWith(inputs.python-version, 'pypy') }}
         if: ${{ steps.ffi-changes.outputs.changed == 'true' && inputs.rust == 'stable' && inputs.python-version != 'pypy-3.7' && inputs.python-version != 'pypy-3.8' }}
 
+
       - name: Test cross compilation
         if: ${{ inputs.os == 'ubuntu-latest' && inputs.python-version == '3.9' }}
         uses: PyO3/maturin-action@v1
@@ -220,4 +218,4 @@ jobs:
       # This is a hack to make CARGO_PRIMARY_PACKAGE always set even for the
       # msrv job. MSRV is currently 1.48, but CARGO_PRIMARY_PACKAGE only came in
       # 1.49.
-      CARGO_PRIMARY_PACKAGE: 1
+      CARGO_PRIMARY_PACKAGE: 1

Cargo.toml

@@ -158,6 +158,8 @@ harness = false
 [workspace]
 members = [
     "pyo3-ffi",
+    "pyo3-ffi-check",
+    "pyo3-ffi-check/macro",
     "pyo3-build-config",
     "pyo3-macros",
     "pyo3-macros-backend",

guide/src/function.md

@@ -73,39 +73,7 @@ The `#[pyo3]` attribute can be used to modify properties of the generated Python
 
   - <a name="text_signature"></a> `#[pyo3(text_signature = "...")]`
 
-    Sets the function signature visible in Python tooling (such as via [`inspect.signature`]).
-
-    The example below creates a function `add` which has a signature describing two positional-only
-    arguments `a` and `b`.
-
-    ```rust
-    use pyo3::prelude::*;
-
-    /// This function adds two unsigned 64-bit integers.
-    #[pyfunction]
-    #[pyo3(text_signature = "(a, b, /)")]
-    fn add(a: u64, b: u64) -> u64 {
-        a + b
-    }
-    #
-    # fn main() -> PyResult<()> {
-    #     Python::with_gil(|py| {
-    #         let fun = pyo3::wrap_pyfunction!(add, py)?;
-    #
-    #         let doc: String = fun.getattr("__doc__")?.extract()?;
-    #         assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
-    #
-    #         let inspect = PyModule::import(py, "inspect")?.getattr("signature")?;
-    #         let sig: String = inspect
-    #             .call1((fun,))?
-    #             .call_method0("__str__")?
-    #             .extract()?;
-    #         assert_eq!(sig, "(a, b, /)");
-    #
-    #         Ok(())
-    #     })
-    # }
-    ```
+    Overrides the PyO3-generated function signature visible in Python tooling (such as via [`inspect.signature`]). See the [corresponding topic in the Function Signatures subchapter](./function/signature.md#making-the-function-signature-available-to-python).
 
   - <a name="pass_module" ></a> `#[pyo3(pass_module)]`
 
@@ -161,47 +129,6 @@ The `#[pyo3]` attribute can be used on individual arguments to modify properties
 
 ## Advanced function patterns
 
-### Making the function signature available to Python (old method)
-
-Alternatively, simply make sure the first line of your docstring is
-formatted like in the following example. Please note that the newline after the
-`--` is mandatory. The `/` signifies the end of positional-only arguments.
-
-`#[pyo3(text_signature)]` should be preferred, since it will override automatically
-generated signatures when those are added in a future version of PyO3.
-
-```rust
-# #![allow(dead_code)]
-use pyo3::prelude::*;
-
-/// add(a, b, /)
-/// --
-///
-/// This function adds two unsigned 64-bit integers.
-#[pyfunction]
-fn add(a: u64, b: u64) -> u64 {
-    a + b
-}
-
-// a function with a signature but without docs. Both blank lines after the `--` are mandatory.
-
-/// sub(a, b, /)
-/// --
-#[pyfunction]
-fn sub(a: u64, b: u64) -> u64 {
-    a - b
-}
-```
-
-When annotated like this, signatures are also correctly displayed in IPython.
-
-```text
->>> pyo3_test.add?
-Signature: pyo3_test.add(a, b, /)
-Docstring: This function adds two unsigned 64-bit integers.
-Type:      builtin_function_or_method
-```
-
 ### Calling Python functions in Rust
 
 You can pass Python `def`'d functions and built-in functions to Rust functions [`PyFunction`]

guide/src/function/signature.md

@@ -188,3 +188,138 @@ impl MyClass {
     }
 }
 ```
+
+## 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.
+
+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]`.
+
+In cases where the automatically-generated signature needs adjusting, it can [be overridden](#overriding-the-generated-signature) using the `#[pyo3(text_signature)]` option.)
+
+The example below creates a function `add` which accepts two positional-only arguments `a` and `b`, where `b` has a default value of zero.
+
+```rust
+use pyo3::prelude::*;
+
+/// This function adds two unsigned 64-bit integers.
+#[pyfunction]
+#[pyo3(signature = (a, b=0, /))]
+fn add(a: u64, b: u64) -> u64 {
+    a + b
+}
+#
+# fn main() -> PyResult<()> {
+#     Python::with_gil(|py| {
+#         let fun = pyo3::wrap_pyfunction!(add, py)?;
+#
+#         let doc: String = fun.getattr("__doc__")?.extract()?;
+#         assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
+#
+#         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, "(a, b=Ellipsis, /)");
+#
+#         Ok(())
+#     })
+# }
+```
+
+The following IPython output demonstrates how this generated signature will be seen from Python tooling:
+
+```text
+>>> pyo3_test.add.__text_signature__
+'(a, b=..., /)'
+>>> pyo3_test.add?
+Signature: pyo3_test.add(a, b=Ellipsis, /)
+Docstring: This function adds two unsigned 64-bit integers.
+Type:      builtin_function_or_method
+```
+
+### Overriding the generated signature
+
+The `#[pyo3(text_signature = "(<some signature>)")]` attribute can be used to override the default generated signature.
+
+In the snippet below, the text signature attribute is used to include the default value of `0` for the argument `b`, instead of the automatically-generated default value of `...`:
+
+```rust
+use pyo3::prelude::*;
+
+/// This function adds two unsigned 64-bit integers.
+#[pyfunction]
+#[pyo3(signature = (a, b=0, /), text_signature = "(a, b=0, /)")]
+fn add(a: u64, b: u64) -> u64 {
+    a + b
+}
+#
+# fn main() -> PyResult<()> {
+#     Python::with_gil(|py| {
+#         let fun = pyo3::wrap_pyfunction!(add, py)?;
+#
+#         let doc: String = fun.getattr("__doc__")?.extract()?;
+#         assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
+#
+#         let inspect = PyModule::import(py, "inspect")?.getattr("signature")?;
+#         let sig: String = inspect
+#             .call1((fun,))?
+#             .call_method0("__str__")?
+#             .extract()?;
+#         assert_eq!(sig, "(a, b=0, /)");
+#
+#         Ok(())
+#     })
+# }
+```
+
+PyO3 will include the contents of the annotation unmodified as the `__text_signature`. Below shows how IPython will now present this (see the default value of 0 for b):
+
+```text
+>>> pyo3_test.add.__text_signature__
+'(a, b=0, /)'
+>>> pyo3_test.add?
+Signature: pyo3_test.add(a, b=0, /)
+Docstring: This function adds two unsigned 64-bit integers.
+Type:      builtin_function_or_method
+```
+
+If no signature is wanted at all, `#[pyo3(text_signature = None)]` will disable the built-in signature. The snippet below demonstrates use of this:
+
+```rust
+use pyo3::prelude::*;
+
+/// This function adds two unsigned 64-bit integers.
+#[pyfunction]
+#[pyo3(signature = (a, b=0, /), text_signature = None)]
+fn add(a: u64, b: u64) -> u64 {
+    a + b
+}
+#
+# fn main() -> PyResult<()> {
+#     Python::with_gil(|py| {
+#         let fun = pyo3::wrap_pyfunction!(add, py)?;
+#
+#         let doc: String = fun.getattr("__doc__")?.extract()?;
+#         assert_eq!(doc, "This function adds two unsigned 64-bit integers.");
+#         assert!(fun.getattr("__text_signature__")?.is_none());
+#
+#         Ok(())
+#     })
+# }
+```
+
+Now the function's `__text_signature__` will be set to `None`, and IPython will not display any signature in the help:
+
+```text
+>>> pyo3_test.add.__text_signature__ == None
+True
+>>> pyo3_test.add?
+Docstring: This function adds two unsigned 64-bit integers.
+Type:      builtin_function_or_method
+```

newsfragments/2784.changed.md

@@ -0,0 +1 @@
+Automatically generate `__text_signature__` for all Python functions created using `#[pyfunction]` and `#[pymethods]`.

noxfile.py

@@ -93,6 +93,9 @@ def clippy(session: nox.Session) -> None:
             f"--features={feature_set}",
             "--all-targets",
             "--workspace",
+            # linting pyo3-ffi-check requires docs to have been built or
+            # the macros will error; doesn't seem worth it on CI
+            "--exclude=pyo3-ffi-check",
             "--",
             "--deny=warnings",
             external=True,
@@ -348,6 +351,12 @@ def check_changelog(session: nox.Session):
         print(fragment.name)
 
 
+@nox.session(name="ffi-check")
+def check_changelog(session: nox.Session):
+    session.run("cargo", "doc", "-p", "pyo3-ffi", "--no-deps", external=True)
+    _run(session, "cargo", "run", "-p", "pyo3-ffi-check", external=True)
+
+
 def _get_rust_target() -> str:
     output = _get_output("rustc", "-vV")
 

pyo3-ffi-check/Cargo.toml

@@ -1,7 +1,7 @@
 [package]
 name = "pyo3-ffi-check"
 version = "0.1.0"
-edition = "2021"
+edition = "2018"
 publish = false
 
 [dependencies]
@@ -10,12 +10,7 @@ memoffset = "0.7.0"
 
 [dependencies.pyo3-ffi]
 path = "../pyo3-ffi"
-features = ["extension-module"]  # A lazy way of skipping linking in most cases (as we don't use any runtime symbols)
-
-[workspace]
-members = [
-    "macro"
-]
+features = ["extension-module"]  # A lazy way of skipping linking in most cases (as we don't use any runtime symbols)
 
 [build-dependencies]
 bindgen = "0.63.0"

pyo3-ffi-check/build.rs

@@ -10,7 +10,6 @@ fn main() {
         .expect("failed to get lib dir");
 
     println!("cargo:rerun-if-changed=wrapper.h");
-    dbg!(format!("-I{python_include_dir}"));
 
     let bindings = bindgen::Builder::default()
         .header("wrapper.h")

pyo3-ffi-check/macro/Cargo.toml

@@ -1,7 +1,8 @@
 [package]
 name = "pyo3-ffi-check-macro"
 version = "0.1.0"
-edition = "2021"
+edition = "2018"
+publish = false
 
 [lib]
 proc-macro = true

pyo3-ffi-check/macro/src/lib.rs

@@ -19,7 +19,7 @@ pub fn for_all_structs(input: proc_macro::TokenStream) -> proc_macro::TokenStrea
         }
     };
 
-    if !input.next().is_none() {
+    if input.next().is_some() {
         return quote!(compile_error!(
             "for_all_structs!() takes only a single ident as input"
         ))
@@ -109,7 +109,7 @@ pub fn for_all_fields(input: proc_macro::TokenStream) -> proc_macro::TokenStream
         }
     };
 
-    if !input.next().is_none() {
+    if input.next().is_some() {
         return quote!(compile_error!(
             "for_all_fields!() takes exactly two idents as input"
         ))

pyo3-ffi-check/src/main.rs

@@ -1,6 +1,13 @@
-use std::process::exit;
+use std::{ffi::CStr, process::exit};
 
 fn main() {
+    println!(
+        "comparing pyo3-ffi against headers generated for {}",
+        CStr::from_bytes_with_nul(bindings::PY_VERSION)
+            .unwrap()
+            .to_string_lossy()
+    );
+
     let mut failed = false;
 
     macro_rules! check_struct {