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

Case for REQUIRED_ARGUMENT_AFTER_OPTION #2925

Closed
ritchie46 opened this issue Jan 28, 2023 · 4 comments
Closed

Case for REQUIRED_ARGUMENT_AFTER_OPTION #2925

ritchie46 opened this issue Jan 28, 2023 · 4 comments

Comments

@ritchie46
Copy link

ritchie46 commented Jan 28, 2023

Hi, while upgrading to 0.18 in polars, I saw this warning:

use of deprecated constant `pyo3::impl_::deprecations::REQUIRED_ARGUMENT_AFTER_OPTION`: required arguments after an `Option<_>` argument are ambiguous and being phased out
         = help: add a `#[pyo3(signature)]` annotation on this function to unambiguously specify the default values for all optional parameters

We have a lot of types in polars where an optional None | int value is valid. There are passed to pyo3 as None in such a case.

We don't use it as non-required argument. Could there be a case where the type Option<T> is still allowed, but it does not mean that the argument can be elided?

Maybe a required_all proc macro?

@adamreichold
Copy link
Member

@ritchie46 Could you please also paste the full signature of such a #[pyfunction] here so hat this easier to understand. Thanks!

@birkenfeld
Copy link
Member

birkenfeld commented Jan 28, 2023

Could there be a case where the type Option is still allowed, but it does not mean that the argument can be elided?

Isn't that the behavior you get when you put the argument into signature without a default value?

I.e. the two cases are

#[pyo3(signature=(a, b))]
fn do(a: u64, b: Option<u64>) {}  // b can be None but must be given from Python

vs

#[pyo3(signature=(a, b=None))]
fn do(a: u64, b: Option<u64>) {}  // b can be omitted from Python

Both cases are still supported, PyO3 just now wants the signature annotation to disambiguate.

@ritchie46
Copy link
Author

Right, I realized later. Maybe defaulting to the any of the two (I oreythw first) might save some proc macro headers.

In any case, that's only ergonomics.

Thanks for the clarification.

@davidhewitt
Copy link
Member

For what it's worth, #2934 may help document this for future readers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants