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

Relax "REQUIRED_ARGUMENT_AFTER_OPTION" deprecation #3735

Open
clbarnes opened this issue Jan 10, 2024 · 1 comment
Open

Relax "REQUIRED_ARGUMENT_AFTER_OPTION" deprecation #3735

clbarnes opened this issue Jan 10, 2024 · 1 comment

Comments

@clbarnes
Copy link

Python's typing.Optional does not mean "may be undefined/ has a default", it means "may be None". Just because an argument is nullable doesn't mean that it has some default. As such, it is perfectly reasonable to have a signature like

def my_fn(a: Optional[int], b: int): ...

However, this is not allowed in pyo3:

pub fn my_fn(a: Option<i64>, b: i64) -> ...
error: required arguments after an `Option<_>` argument are ambiguous
       = help: add a `#[pyo3(signature)]` annotation on this function to unambiguously specify the default values for all optional parameters

I'd argue that required arguments after an Option<_> are not ambiguous. As far as I can tell, there is now no way to represent my first case in pyo3, because adding a signature annotation would only allow me to specify a default, which I do not have.

Is it possible to relax this and allow that first case to be represented in the way A) it used to be, and B) is the obvious equivalent between the rust and python representations?

clbarnes added a commit to clbarnes/ncollpyde that referenced this issue Jan 10, 2024
switch to pseudonormal containment check is now n_rays=0 rather than
None, due to PyO3/pyo3#3735
@davidhewitt
Copy link
Member

You can represent it like this, making the option required:

#[pyo3(signature = (a, b))]
fn my_fn(a: Option<i64>, b: i64)

For what it's worth, I completely agree with you on this point that Optional does not imply a default. However, PyO3 made a historical design choice to automatically infer all trailing Option arguments have a default of None.

I've proposed to remove this previously, see #2935

There's enough churn in 0.21 that I wouldn't want to proceed with that change right now but I still believe it's the right choice. I suggest you read that thread and if you agree, voice your support there.

Removing that automatic default would also mean that I would remove this error message here which you have encountered. The problem at the moment is that this function has an implicit default of None for a:

fn my_fn(a: Option<i64>) -> ...

but by adding b, because a would no longer be trailing, it doesn't get the implicit default any more. I wanted to avoid this silently breaking user code, hence the error message.

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

No branches or pull requests

2 participants