pyo3
pyo3 copied to clipboard
RFC: remove default of None for trailing optional arguments
In #2934 I've documented a longstanding PyO3 behaviour to automatically add a default of None to all trailing Option<T> arguments.
I think that behaviour is a small footgun, as I don't think first-time users necessarily expect this. I see two downsides:
- Users accidentally allow default of
Nonefor these arguments when they wanted them to be required - Users reordering function arguments can easily remove the default without noticing.
We currently have the deprecation from #2703 which we can upgrade to an error in 0.20. This helps the situation, as it makes the refactoring in the second bullet fail nosily.
I'd be keen to remove the trailing default, with a deprecation warning for functions using this behaviour which don't currently have a #[pyo3(signature = (...))] annotation. I think it's a simpler API for users if PyO3 never adds an implicit default, and removes the need for the error on the refactoring case (which is purely a guard rail to avoid the current footgun).
What do others think? I'd be keen to hear from folks who think the conveniences of the current behaviour (i.e. fewer macro annotations) outweigh the drawbacks listed above. (@mejrs - in #2193 I got the impression that you might hold this view.)
IMO the current mode (with the warning elevated to error) is fine. With arguments that can be None, it's pretty typical that they also have a default of None.
@ritchie46 - after reading #2925 I'd be interested in your comment as a user - you were either unaware of this default, or would prefer the change proposed here?
#2932 sounds like another case of a user confused by this (maybe just because it wasn't documented).
I think it's a simpler API for users if PyO3 never adds an implicit default
I'm generally against doing implicit things, so this sounds good.
I've been mulling about how to communicate this behavior. The current api heavily relies on assuming that users read the documentation. I've been guilty of this myself - I have at times just used Option without really thinking about what behavior I actually wanted. What about just requiring the signature attribute if there is an Option anywhere in the function signature?
I'm generally against doing implicit things, so this sounds good.
Although "implicit" can mean a lot of different things.
In this case, you could argue that any case of not using pyo3(signature) is implicit, and pyo3(signature) should be mandatory.
@ritchie46 - after reading #2925 I'd be interested in your comment as a user - you were either unaware of this default, or would prefer the change proposed here?
I was unaware of the defaults. I think having a well documented default makes sense. It can remove some boilerplate for the default case.
In this case, you could argue that any case of not using
pyo3(signature)is implicit, andpyo3(signature)should be mandatory.
Definitely true, though I'd make the argument that it's reasonable to assume that not using pyo3(signature) would be the same as pyo3(signature = (x, y, z)), i.e. the trivial signature with no special modifications.
Because of the rule here which I propose we deprecate, at the moment the implicit signature is more like pyo3(signature = (x, y, z=None)), with the number of =None varying depending on how many trailing Option<T> arguments there are.
I was unaware of the defaults. I think having a well documented default makes sense. It can remove some boilerplate for the default case.
Thanks. Good to know.
From a user perspective, for some function like
#[pyfunction]
fn my_function(arg1: Option<String>, arg2: Option<String>) {
depending on the default we pick here, one of the following forms will always be necessary:
// to take required `Option<String>` arguments, this is currently necessary
#[pyfunction(signature = (arg1, arg2))]
fn my_function(arg1: Option<String>, arg2: Option<String>) {
// to make optional arguments, this would be necessary if the proposal here were accepted
#[pyfunction(signature = (arg1=None, arg2=None))]
fn my_function(arg1: Option<String>, arg2: Option<String>) {
The question is which of these would users be happier writing?
Personally, I think it's more frustrating as a user to have to write the form #[pyfunction(signature = (arg1, arg2))]. That looks and feels like a no-op to me which forces me to repeat myself. Writing the latter form #[pyfunction(signature = (arg1=None, arg2=None))], by contrast, is making the defaults of None very explicit, which is not otherwise obvious from the Rust function definition, so I think helps readability even if it's a little boilerplatey.
What about just requiring the signature attribute if there is an
Optionanywhere in the function signature?
I think to action the change here, we would need to do exactly that for a couple of versions (maybe as a deprecation warning). In the long term, I'd prefer to not have to write the "no-op" signature (e.g. #[pyfunction(signature = (arg1, arg2))]).
Add me to the "confused user" list (#3735 ). Rust does not have a concept of optional arguments, just arguments of type Option. Python has the concept of optional arguments and arguments of type Optional, and crucially, those are different things. The current behaviour conflates the two by trying to pre-empt user behaviour in a way I don't find particularly convincing. It's true that sometimes nullable arguments are null by default, but there are plenty of cases where they're not, and IMO the default behaviour should be to rely on an explicit distinction between what is optional and what is nullable, rather than having to add extra information to tell pyo3 code to behave like rust code and python code (python does not add an implicit =None to arg: Optional either).
My particular case is that
# python
def my_fn(a: Optional[int], b: int): ...
has the obvious equivalent of
// pyo3
pub fn my_fn(a: Option<i64>, b: i64) -> ...
but the latter is a compile error.