Make stacklevel=2 by default in warnings.warn()
For now the default value of the stacklevel parameter in warnings.warn() is 1. But in most cases stacklevel=2 is required, sometimes >2, and I don't know cases that need stacklevel=1. I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code. But rather can fix existing bugs. If stacklevel=1 is required (I don't know cases), it can be explicitly specified.
On Sat, Sep 19, 2015 at 11:44 PM, Serhiy Storchaka <storchaka@gmail.com> wrote:
For now the default value of the stacklevel parameter in warnings.warn() is 1. But in most cases stacklevel=2 is required, sometimes >2, and I don't know cases that need stacklevel=1. I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code. But rather can fix existing bugs. If stacklevel=1 is required (I don't know cases), it can be explicitly specified.
+1 I don't have enough fingers to count how many times I've had to explain how stacklevel= works to maintainers of widely-used packages -- they had no idea that this was even a thing they were getting wrong. OTOH I guess if there is anyone out there who's intentionally using stacklevel=1 they might be reasonably surprised at this change. I guess for some kinds of warnings stacklevel=2 is not obviously correct -- the one that comes to mind is "warning: the computer on the other end of this network connection did something weird, continuing anyway". OTOOH in this case I'm not sure stacklevel=1 is any better either, since the thing being warned about has nothing to do with the current call stack at all. -n -- Nathaniel J. Smith -- http://vorpus.org
субота, 19-вер-2015 23:55:44 Nathaniel Smith написано:
OTOH I guess if there is anyone out there who's intentionally using stacklevel=1 they might be reasonably surprised at this change.
That is why I ask on Python-Dev instead of just open an issue. But I doubt that there is such case.
On 20 September 2015 at 07:55, Nathaniel Smith <njs@pobox.com> wrote:
On Sat, Sep 19, 2015 at 11:44 PM, Serhiy Storchaka <storchaka@gmail.com> wrote:
For now the default value of the stacklevel parameter in warnings.warn() is 1. But in most cases stacklevel=2 is required, sometimes >2, and I don't know cases that need stacklevel=1. I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code. But rather can fix existing bugs. If stacklevel=1 is required (I don't know cases), it can be explicitly specified.
+1
I don't have enough fingers to count how many times I've had to explain how stacklevel= works to maintainers of widely-used packages -- they had no idea that this was even a thing they were getting wrong.
OTOH I guess if there is anyone out there who's intentionally using stacklevel=1 they might be reasonably surprised at this change. I guess for some kinds of warnings stacklevel=2 is not obviously correct -- the one that comes to mind is "warning: the computer on the other end of this network connection did something weird, continuing anyway". OTOOH in this case I'm not sure stacklevel=1 is any better either, since the thing being warned about has nothing to do with the current call stack at all.
In this case you should use the logging module instead. -- Gustavo J. A. M. Carneiro Gambit Research "The universe is always one step beyond logic." -- Frank Herbert
Would it be too verbose to display two frames or more by default? Maybe depending on the action (ex: only if the warning is emitted only once). Victor 2015-09-20 8:44 GMT+02:00 Serhiy Storchaka <storchaka@gmail.com>:
For now the default value of the stacklevel parameter in warnings.warn() is 1. But in most cases stacklevel=2 is required, sometimes >2, and I don't know cases that need stacklevel=1. I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code. But rather can fix existing bugs. If stacklevel=1 is required (I don't know cases), it can be explicitly specified.
_______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/victor.stinner%40gmail.co...
On Sep 21, 2015 12:15 AM, "Victor Stinner" <victor.stinner@gmail.com> wrote:
Would it be too verbose to display two frames or more by default? Maybe depending on the action (ex: only if the warning is emitted only once).
It's not just about how it gets displayed -- the frame that it gets "attributed" to is also used when comparing against the warnings filters to determine what action to take. What if stacklevel=1 makes it match a filter with action "ignore", and stacklevel=2 makes it match a filter with action "raise"? (This is a common example I've encountered in the context of wanting to set up CI tests so that if *my* code uses deprecated functionality then I want to fail the test so I notice, but if some library I'm using uses deprecated functionality internally then that's not my problem. This breaks down when the library makes the common error of issuing DeprecationWarnings with stacklevel=1, because that makes python think that they are deprecating themselves, not warning that I did something deprecated.) -n
On 21 September 2015 at 17:21, Nathaniel Smith <njs@pobox.com> wrote:
On Sep 21, 2015 12:15 AM, "Victor Stinner" <victor.stinner@gmail.com> wrote:
Would it be too verbose to display two frames or more by default? Maybe depending on the action (ex: only if the warning is emitted only once).
It's not just about how it gets displayed -- the frame that it gets "attributed" to is also used when comparing against the warnings filters to determine what action to take. What if stacklevel=1 makes it match a filter with action "ignore", and stacklevel=2 makes it match a filter with action "raise"?
(This is a common example I've encountered in the context of wanting to set up CI tests so that if *my* code uses deprecated functionality then I want to fail the test so I notice, but if some library I'm using uses deprecated functionality internally then that's not my problem. This breaks down when the library makes the common error of issuing DeprecationWarnings with stacklevel=1, because that makes python think that they are deprecating themselves, not warning that I did something deprecated.)
As Victor notes, though, that's not the right default for *scripts* that are issuing deprecation warnings to end users - there, they really are deprecating themselves. It's also not the right default for pretty much any warning other than DeprecationWarning - for those, you're generally wanting to say "we hit this problematic code path", not report anything about your caller. Passing "stacklevel=2" for API deprecations is by no means obvious though, so perhaps it makes sense to add a "warnings.warn_deprecated(message)" function that's implemented as: def warn_deprecated(message, stacklevel=1): return warnings.warn(message, DeprecationWarning, stacklevel=(2+stacklevel)). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
I think it's been conclusively shown that we should not change the default. Instead I recommend updating the docs with an example showing when stacklevel=2 is appropriate. On Mon, Sep 21, 2015 at 2:05 AM, Nick Coghlan <ncoghlan@gmail.com> wrote:
On 21 September 2015 at 17:21, Nathaniel Smith <njs@pobox.com> wrote:
On Sep 21, 2015 12:15 AM, "Victor Stinner" <victor.stinner@gmail.com> wrote:
Would it be too verbose to display two frames or more by default? Maybe depending on the action (ex: only if the warning is emitted only once).
It's not just about how it gets displayed -- the frame that it gets "attributed" to is also used when comparing against the warnings filters to determine what action to take. What if stacklevel=1 makes it match a filter with action "ignore", and stacklevel=2 makes it match a filter with action "raise"?
(This is a common example I've encountered in the context of wanting to set up CI tests so that if *my* code uses deprecated functionality then I want to fail the test so I notice, but if some library I'm using uses deprecated functionality internally then that's not my problem. This breaks down when the library makes the common error of issuing DeprecationWarnings with stacklevel=1, because that makes python think that they are deprecating themselves, not warning that I did something deprecated.)
As Victor notes, though, that's not the right default for *scripts* that are issuing deprecation warnings to end users - there, they really are deprecating themselves. It's also not the right default for pretty much any warning other than DeprecationWarning - for those, you're generally wanting to say "we hit this problematic code path", not report anything about your caller.
Passing "stacklevel=2" for API deprecations is by no means obvious though, so perhaps it makes sense to add a "warnings.warn_deprecated(message)" function that's implemented as:
def warn_deprecated(message, stacklevel=1): return warnings.warn(message, DeprecationWarning, stacklevel=(2+stacklevel)).
Cheers, Nick.
-- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia _______________________________________________ Python-Dev mailing list Python-Dev@python.org https://mail.python.org/mailman/listinfo/python-dev Unsubscribe: https://mail.python.org/mailman/options/python-dev/guido%40python.org
-- --Guido van Rossum (python.org/~guido)
On 21.09.15 12:05, Nick Coghlan wrote:
As Victor notes, though, that's not the right default for *scripts* that are issuing deprecation warnings to end users - there, they really are deprecating themselves. It's also not the right default for pretty much any warning other than DeprecationWarning - for those, you're generally wanting to say "we hit this problematic code path", not report anything about your caller.
If a warning is emitted in library function, user can't do anything with function's code and should change his code that calls the function. In this case the warning should point to the line where the function is used. If a warning is emitted in user's code, he is able to add explicit stacklevel=1 if needed. stacklevel=2 is correct argument for module level warnings, but as Nathaniel noted not all packages use it. When your run correctly written module as script you get the same "sys:1:". And this issue can be solved in general case. not allecause the user wants to get a warning at the place where the module is imported.
Passing "stacklevel=2" for API deprecations is by no means obvious though, so perhaps it makes sense to add a "warnings.warn_deprecated(message)" function that's implemented as:
def warn_deprecated(message, stacklevel=1): return warnings.warn(message, DeprecationWarning, stacklevel=(2+stacklevel)).
This will not fix tons of modules that are not aware of correct stacklevel.
On Mon, 21 Sep 2015 at 10:15 Serhiy Storchaka <storchaka@gmail.com> wrote:
On 21.09.15 12:05, Nick Coghlan wrote:
As Victor notes, though, that's not the right default for *scripts* that are issuing deprecation warnings to end users - there, they really are deprecating themselves. It's also not the right default for pretty much any warning other than DeprecationWarning - for those, you're generally wanting to say "we hit this problematic code path", not report anything about your caller.
If a warning is emitted in library function, user can't do anything with function's code and should change his code that calls the function. In this case the warning should point to the line where the function is used.
If a warning is emitted in user's code, he is able to add explicit stacklevel=1 if needed.
stacklevel=2 is correct argument for module level warnings, but as Nathaniel noted not all packages use it. When your run correctly written module as script you get the same "sys:1:". And this issue can be solved in general case.
not allecause the user wants to get a warning at the place where the module is imported.
Passing "stacklevel=2" for API deprecations is by no means obvious though, so perhaps it makes sense to add a "warnings.warn_deprecated(message)" function that's implemented as:
def warn_deprecated(message, stacklevel=1): return warnings.warn(message, DeprecationWarning, stacklevel=(2+stacklevel)).
This will not fix tons of modules that are not aware of correct stacklevel.
The long-term solution to this is to add a warnings.deprecate_module() function which does the right thing and simply not expose the stacklevel argument to users. We can then consider warnings.warn() more of a power user function and make the stacklevel a keyword-only argument with no default value (although I can already hear Raymond saying "but the Python 2 users" for that part of the idea =).
On 21.09.15 20:32, Brett Cannon wrote:
On Mon, 21 Sep 2015 at 10:15 Serhiy Storchaka <storchaka@gmail.com <mailto:storchaka@gmail.com>> wrote:
On 21.09.15 12:05, Nick Coghlan wrote: > Passing "stacklevel=2" for API deprecations is by no means obvious > though, so perhaps it makes sense to add a > "warnings.warn_deprecated(message)" function that's implemented as: > > def warn_deprecated(message, stacklevel=1): > return warnings.warn(message, DeprecationWarning, > stacklevel=(2+stacklevel)).
This will not fix tons of modules that are not aware of correct stacklevel.
The long-term solution to this is to add a warnings.deprecate_module() function which does the right thing and simply not expose the stacklevel argument to users. We can then consider warnings.warn() more of a power user function and make the stacklevel a keyword-only argument with no default value (although I can already hear Raymond saying "but the Python 2 users" for that part of the idea =).
warnings.deprecate_module() will do the right thing only for deprecated modules, but most deprecations refer to separate functions.
On Mon, 21 Sep 2015 at 11:29 Serhiy Storchaka <storchaka@gmail.com> wrote:
On 21.09.15 20:32, Brett Cannon wrote:
On Mon, 21 Sep 2015 at 10:15 Serhiy Storchaka <storchaka@gmail.com <mailto:storchaka@gmail.com>> wrote:
On 21.09.15 12:05, Nick Coghlan wrote: > Passing "stacklevel=2" for API deprecations is by no means obvious > though, so perhaps it makes sense to add a > "warnings.warn_deprecated(message)" function that's implemented
as:
> > def warn_deprecated(message, stacklevel=1): > return warnings.warn(message, DeprecationWarning, > stacklevel=(2+stacklevel)).
This will not fix tons of modules that are not aware of correct stacklevel.
The long-term solution to this is to add a warnings.deprecate_module() function which does the right thing and simply not expose the stacklevel argument to users. We can then consider warnings.warn() more of a power user function and make the stacklevel a keyword-only argument with no default value (although I can already hear Raymond saying "but the Python 2 users" for that part of the idea =).
warnings.deprecate_module() will do the right thing only for deprecated modules, but most deprecations refer to separate functions.
And we can add one for functions as well. My point is that people seem to not fully comprehend how to properly use the stacklevel argument and so I'm suggesting we have functions for the common cases (modules, functions, methods, classes).
On Mon, Sep 21, 2015 at 10:14 AM, Serhiy Storchaka <storchaka@gmail.com> wrote:
stacklevel=2 is correct argument for module level warnings, but as Nathaniel noted not all packages use it. When your run correctly written module as script you get the same "sys:1:". And this issue can be solved in general case.
Point of information: for module deprecations, stacklevel=2 is correct in 2.7 and 3.5, but in 3.4 the correct value is stacklevel=8 (see https://bugs.python.org/issue24305#msg249981). I'm inclined to think that this is a bug that should be fixed in 3.4.4, but Larry disagrees and I don't really care enough to argue about it either way :-). -n -- Nathaniel J. Smith -- http://vorpus.org
On 22 Sep 2015 03:16, "Serhiy Storchaka" <storchaka@gmail.com> wrote:
On 21.09.15 12:05, Nick Coghlan wrote:
As Victor notes, though, that's not the right default for *scripts* that are issuing deprecation warnings to end users - there, they really are deprecating themselves. It's also not the right default for pretty much any warning other than DeprecationWarning - for those, you're generally wanting to say "we hit this problematic code path", not report anything about your caller.
If a warning is emitted in library function, user can't do anything with
function's code and should change his code that calls the function. In this case the warning should point to the line where the function is used.
If a warning is emitted in user's code, he is able to add explicit
stacklevel=1 if needed.
stacklevel=2 is correct argument for module level warnings, but as
Nathaniel noted not all packages use it. When your run correctly written module as script you get the same "sys:1:". And this issue can be solved in general case. Aye, combined with your suggestion of not going beyond the top of the stack when issuing warnings, I've been persuaded that changing the default stack level to 2 makes sense. The other relevant thing I recalled is the guidance in https://docs.python.org/3/howto/logging.html#logging-basic-tutorial, which suggests using warnings.warn if you want the caller to change their code, logging.warn otherwise. Cheers, Nick.
2015-09-20 8:44 GMT+02:00 Serhiy Storchaka <storchaka@gmail.com>:
I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code.
Consider this simple script: --- import warnings warnings.warn("here") --- Currrent output: --- x.py:3: UserWarning: here warnings.warn("here") --- => it shows the script name (x.py), the line number and the line, as expected. Now try stacklevel=2: --- import warnings warnings.warn("here", stacklevel=2) --- New output: --- sys:1: UserWarning: here --- "sys:1" is not really useful :-/ I would describe this as a regression, not an enhancement. It's hard to find a "good" default value. It's better to always specify stacklevel :-) Victor
On 21 Sep 2015, at 9:18, Victor Stinner wrote:
2015-09-20 8:44 GMT+02:00 Serhiy Storchaka <storchaka@gmail.com>:
I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code.
Consider this simple script: --- import warnings warnings.warn("here") ---
Currrent output: --- x.py:3: UserWarning: here warnings.warn("here") ---
=> it shows the script name (x.py), the line number and the line, as expected.
Now try stacklevel=2: --- import warnings warnings.warn("here", stacklevel=2) ---
New output: --- sys:1: UserWarning: here ---
"sys:1" is not really useful :-/
I would describe this as a regression, not an enhancement.
It's hard to find a "good" default value. It's better to always specify stacklevel :-)
A "dynamic" stacklevel might help. Normally when you implement a call to warning.warn() inside a module, you want to report the innermost stacklevel that is outside of your module, because that's the spot where the error likely is. This could be done automatically (report the first module that is different from the one where the call to warning.warn() is), or by specifying a base package name or regular expression, i.e. report the innermost stackframe that is not from "mypackage.mysubpackage"). See http://bugs.python.org/issue850482 Bye, Walter Dörwald
On 21.09.15 10:18, Victor Stinner wrote:
2015-09-20 8:44 GMT+02:00 Serhiy Storchaka <storchaka@gmail.com>:
I propose to make the default value of stacklevel to be 2. I think that unlikely this will break existing code.
Consider this simple script: --- import warnings warnings.warn("here") ---
Currrent output: --- x.py:3: UserWarning: here warnings.warn("here") ---
=> it shows the script name (x.py), the line number and the line, as expected.
Now try stacklevel=2: --- import warnings warnings.warn("here", stacklevel=2) ---
New output: --- sys:1: UserWarning: here ---
"sys:1" is not really useful :-/
This is not new. The same output we get when run a module that correctly emits a warning at module level (with explicit stacklevel=2). $ ./python -Wa Lib/imp.py sys:1: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses If this message looks confusing for you, we can avoid it if stop skipping frames further if frame.f_back is None. This will got rid of "sys:1:" in both cases, explicit and implicit stacklevel=2.
On 21.09.15 20:00, Serhiy Storchaka wrote:
On 21.09.15 10:18, Victor Stinner wrote:
--- sys:1: UserWarning: here ---
"sys:1" is not really useful :-/
This is not new. The same output we get when run a module that correctly emits a warning at module level (with explicit stacklevel=2).
$ ./python -Wa Lib/imp.py sys:1: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
If this message looks confusing for you, we can avoid it if stop skipping frames further if frame.f_back is None. This will got rid of "sys:1:" in both cases, explicit and implicit stacklevel=2.
The patch is provided in http://bugs.python.org/issue25204.
participants (8)
-
Brett Cannon -
Guido van Rossum -
Gustavo Carneiro -
Nathaniel Smith -
Nick Coghlan -
Serhiy Storchaka -
Victor Stinner -
Walter Dörwald