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

'Deprecation of implicit default for trailing optional arguments' Causing issues with erasable setters #4292

Closed
SuperJappie08 opened this issue Jun 26, 2024 · 1 comment · Fixed by #4304
Labels

Comments

@SuperJappie08
Copy link
Contributor

SuperJappie08 commented Jun 26, 2024

Bug Description

As a result of the (upcoming) changes in 0.22 and 0.23 with Option<T> arguments requiring a #[pyo3(signature=...)] link to migration guide.

This causes a problem for setters of a pyclass, which can be erased by setting None, since you're not allowed to specify a signature on a setter.

This is caused by #2935. In this case, I do not need a default argument.
However, it will be required in version 0.23.

Steps to Reproduce

use pyo3::prelude::*;

#[pyclass]
struct SimpleClass {
    field: Option<u32>
}

#[pymethods]
impl SimpleClass {
    #[new]
    #[pyo3(signature = (value=None))]
    fn new(field: Option<u32>) -> Self {
        Self { field }
    }

    #[getter]
    fn get_field(&self) -> Option<u32> {
        self.field
    }

    /// This causes issues
    #[setter]
    fn set_field((&mut self, field: Option<u32>) {
        self.field = field;
    }
}

Backtrace

No response

Your operating system and version

Windows/Linux (All effected)

Your Python version (python --version)

Python 3.10.12

Your Rust version (rustc --version)

rustc 1.79.0 (129f3b996 2024-06-10)

Your PyO3 version

0.22+

How did you install python? Did you use a virtualenv?

apt + venv

Additional Info

This currently causes non-removable warnings, however in the future this will break code.

EDIT: Added more information

@SuperJappie08
Copy link
Contributor Author

After some digging, I found a possible solution, which is to check the function type to not be a setter method here.

if spec.signature.attribute.is_none()
&& spec.signature.arguments.iter().any(|arg| {
if let FnArg::Regular(arg) = arg {
arg.option_wrapped_type.is_some()
} else {
false
}
})

This can probably be done by adding && !matches!(spec.tp, FnType::Setter(_)).

But I am not sure if an error will occur in 0.23 as a result of something somewhere else.

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

Successfully merging a pull request may close this issue.

1 participant