Fix test_decorator issue #269 #270
Conversation
|
|
|
Added a |
| [ | ||
| ("", 1, partial(_fn_with_positional_only, 1)), | ||
| ("2", 2, partial(_fn_with_positional_only, 1)), | ||
| ("2", 2, _fn_with_positional_only), | ||
| ("2", 2, partial(_fn_with_positional_only)), |
There was a problem hiding this comment.
Hey there @zhiruiluo , Would you mind explaining a few things:
- why is the
delay_wrappernecessary here? - Did the
inspectmodule change from 3.11 to 3.11.3? Is that what broke this test?
There was a problem hiding this comment.
The recently PR python/cpython#103557 disallows pos-or-kw params without default after pos-only with default, which breaks the test for Python 3.11.4 released on June 6.
There was a problem hiding this comment.
They will likely backport this change to all Python 3.11 verions.
There was a problem hiding this comment.
Thanks for your reply @lebrice. The delay_wrapper can postpone the generation of partial with changing default function until test collected. If the error happens in pytest collection time, we can't use xfail to bypass the error.
There was a problem hiding this comment.
I double checked the backport status. This PR python/cpython#103675 has backport inspect.Signature change to Python 3.11
There was a problem hiding this comment.
I don't really understand the change in the inspect module, and I also don't quite understand your changes here either..
I do understand that you want to delay the call to change_signature(fn_with_all_argument_types, 1, c=1) because executing it in Python 3.11.4 raises a ValueError("non-default argument follows default argument").
However, before your change, partial here is meant as change_signature, it makes a variant of a function with a different signature (different default values in the signature).
Here your delayed_wrapper is exactly the same as functools.partial, and when applied to the partial (a.k.a. change_signature, it doesn't change anything, as far as I can tell. Why are you applying it to the change_signature function?
It also seems to my eyes like you are changing the behaviour of the rest of the test cases by only adding this additional level of "delay" to only some of the test cases.
I'm going to re-read your PR, but I'd really appreciate if you could clarify this a bit for me.
|
Closed in favour of #273 |
To fix issue #269.