-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Fixed#2684: use the correct stacklevel for deprecation warnings #2689
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
Conversation
@@ -636,7 +636,7 @@ def __init__(self, *names, **attrs): | |||
'pytest now uses argparse. "%default" should be' | |||
' changed to "%(default)s" ', | |||
DeprecationWarning, | |||
stacklevel=3) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this change looks incorrect, the argument type is used indirectly and the higher level intended to trigger that warning for the method call side
@@ -919,7 +919,7 @@ def _idval(val, argname, idx, idfn, config=None): | |||
import warnings | |||
msg = "Raised while trying to determine id of parameter %s at position %d." % (argname, idx) | |||
msg += '\nUpdate your code as this will raise an error in pytest-4.0.' | |||
warnings.warn(msg, DeprecationWarning) | |||
warnings.warn(msg, DeprecationWarning, stacklevel=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
for this one stacklevel is actually not applicable
@@ -63,7 +63,7 @@ def extract_from(cls, parameterset, legacy_force_tuple=False): | |||
argval = argval, | |||
|
|||
if newmarks: | |||
warnings.warn(MARK_PARAMETERSET_UNPACKING) | |||
warnings.warn(MARK_PARAMETERSET_UNPACKING, stacklevel=2) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in this case stacklevel is also not usable, its a case where its probably more sensible to warn explicit and pass the test around
i should have been more clear with what i meant by review, because simply setting |
I agree, unfortunately a few of those will need to use |
I am closing this. |
Thanks anyway for the effort @srinivasreddy, we appreciate it! 👍 |
No description provided.