Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

add #[pyo3(signature = (...))] attribute #2702

Merged
merged 3 commits into from
Oct 25, 2022
Merged

Conversation

davidhewitt
Copy link
Member

This is an implementation of the proposal in #2193 to add a new #[pyo3(signature = (...))] attribute to specify function arguments.

This PR also deprecates the old way of setting function argument signatures.

Compared to #[pyfunction(...)] / #[args(...)]:

  • It is more consistent: always signature = (...) compared to different forms for #[pyfunction] and #[pymethods].
  • It is more strictly validated: all arguments must appear in the signature in the correct order.
  • It has more Pythonic syntax, e.g.:
    • *args instead of args = "*".
    • Default arguments don't need additional string quoting - signature(name = "David") instead of args(name = "\"David\"")

The possible argument I can see which could be made against this is the increased verbosity from requiring all arguments to appear. I argue this is a plus to ensure correctness. It would be easy to relax this in future if feedback frequently requested it.

Future extensions:

  • I'd like this to also generate a text_signature automatically
  • I'd like this to eventually support Python type annotation syntax e.g. signature = (a: int = 5, b: str = "hello"), although the mechanism how the type annotations would be exposed is still unclear.

To review this, I suggest looking at the guide and tests, and if you're satisfied with that give the implementation a scan.

@adamreichold
Copy link
Member

The issue number of the news fragments (2302) does not match the PR issue number (2702) which is why that CI job fails.

@davidhewitt
Copy link
Member Author

Thanks for the review - addressed both comments.

Copy link
Member

@birkenfeld birkenfeld left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect, the new syntax is so much nicer!

guide/src/function/signature.md Show resolved Hide resolved
pytests/src/datetime.rs Outdated Show resolved Hide resolved
@davidhewitt davidhewitt merged commit 8e8b484 into PyO3:main Oct 25, 2022
@davidhewitt davidhewitt deleted the signature branch October 25, 2022 06:23
bors bot added a commit that referenced this pull request Nov 22, 2022
2739: error when `#[pyo3(signature = ())]` used on invalid methods r=davidhewitt a=davidhewitt

A follow-up to #2702 to reject some invalid applications of `#[pyo3(signature = (...))]` attribute, specifically on magic methods and getters / setters / class attributes.

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
bors bot added a commit that referenced this pull request Jan 15, 2023
2703: deprecate required argument after `Option<T>` without signature r=davidhewitt a=davidhewitt

This PR is a follow-up to #2702 to make required arguments after `Option<T>` arguments require a `#[pyo3(signature)]` annotation to remove the possible ambiguity on whether the developer actually wanted to have a required option (which is what PyO3 is currently forced to assume).

Co-authored-by: David Hewitt <1939362+davidhewitt@users.noreply.github.com>
mr-eyes added a commit to sourmash-bio/sourmash_plugin_branchwater that referenced this pull request Jul 24, 2024
mr-eyes added a commit to sourmash-bio/sourmash_plugin_branchwater that referenced this pull request Aug 17, 2024
* following pyp3 sig from PyO3/pyo3#2702

* initialy working

* writing to a sourmash sig instead

* now faster

* fix

* [WIP] optional save-matches

* [DONE] optional save-matches fastmultigather

* add a simple test

* fix name

* doc

* f,t

* Remove test line

* resolve comments

* resolve comments

* remove testing line

* MRG: update `--save-matches` code a bit. (#420)

* add pyo3 decoration

* do not ignore result

* fix fmt

* remove redundant

* MRG: more updates for `--save-matches` (#423)

* make use of in_directory

* add more tests

---------

Co-authored-by: C. Titus Brown <titus@idyll.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants