-
Notifications
You must be signed in to change notification settings - Fork 770
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
docs: Precise the abscense of py: Python
for the #[pyo3(signature)]
#2929
docs: Precise the abscense of py: Python
for the #[pyo3(signature)]
#2929
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, good to have this in documentation! It also inspired me to add #2930 to further help users.
I just have one suggestion on wording here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me, this doesn't compile though 🙂
Just one thing 🙂 error[E0425]: cannot find function, tuple struct or tuple variant `OK` in this scope
--> src/lib.rs:561:5
|
16 | OK(())
| ^^ help: a tuple variant with a similar name exists (notice the capitalization): `Ok` |
2930: add better error message for Python in signature r=adamreichold a=davidhewitt Inspired by #2929, this just adds a better error message when `Python` arguments are accidentally included in the signature. Co-authored-by: David Hewitt <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, this looks great to me! I'll force-push this to squash, and then merge.
2652cb7
to
9306d56
Compare
bors r+ |
Build succeeded: |
Thank you @davidhewitt ! 🙌 |
Hi,
First, thank you for working on PyO3!
I think this adds a precision which was not obvious when migrating to 0.18.0 conventions.
What do you think? Also, should something be added to the migration guide in this regard?
Thank you!