Add additional docs for uncooperative ctor deprecation #9498
Conversation
for more information, see https://pre-commit.ci
| def __init__(self, name, parent, spec): | ||
| super().__init__(name, parent) | ||
| def __init__(self, spec, **kwargs): | ||
| super().__init__(**kwargs) |
There was a problem hiding this comment.
@RonnyPfannschmidt would you agree that we should use and recommend **kwargs here too (despite no conflict at the moment), so that we can freely change these arguments if needed in the future?
If so, I wonder if we should always show this deprecation warning if **kwargs are missing, rather than only showing it in case there are conflicts?
There was a problem hiding this comment.
the warning was introduced to handle broken code
if we introduce it like this, then we should also take a look at standardizing to kwonly args so we can at some point deprecate positional args to nodes
doc/en/deprecations.rst
Outdated
| .. code-block:: python | ||
|
|
||
| class CustomItem(pytest.Item): | ||
| def __init__(self, additional_arg, **kwargs): |
There was a problem hiding this comment.
| def __init__(self, additional_arg, **kwargs): | |
| def __init__(self, *, additional_arg, **kwargs): |
There was a problem hiding this comment.
Hmm, I'm not sure about this honestly - while I agree it's a good thing in general, in the context of this deprecation warning, it seems more confusing than useful, no?
There was a problem hiding this comment.
I agree with @RonnyPfannschmidt, I think we should encourage using kwarg-only in the ctors.
The-Compiler
added
the
needs backport
doc/en/deprecations.rst
Outdated
| .. code-block:: python | ||
|
|
||
| class CustomItem(pytest.Item): | ||
| def __init__(self, additional_arg, **kwargs): |
There was a problem hiding this comment.
I agree with @RonnyPfannschmidt, I think we should encourage using kwarg-only in the ctors.
doc/en/example/nonpython/conftest.py
Outdated
| @@ -18,8 +18,8 @@ def collect(self): | |||
|
|
|||
|
|
|||
| class YamlItem(pytest.Item): | |||
| def __init__(self, name, parent, spec): | |||
| super().__init__(name, parent) | |||
| def __init__(self, spec, **kwargs): | |||
There was a problem hiding this comment.
Following discussion above:
| def __init__(self, spec, **kwargs): | |
| def __init__(self, *, spec, **kwargs): |
|
Apologies for the delay, I forgot about this. Updated now. |
Fixes #9488
Probably still needs test adjustments, but I'd like some feedback from @RonnyPfannschmidt if this is the way to go.