Suboptimal stacklevel of deprecation warnings for formatter and imp modules

[Arfrever]

BPO	23810
Nosy	@brettcannon, @ncoghlan, @larryhastings, @ericsnowcurrently, @berkerpeksag, @serhiy-storchaka
Superseder	bpo-24305: The new import system makes it inconvenient to correctly issue a deprecation warning for a module
Files	better_stacklevel.diff deprecated_module_stacklevel.diff

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/brettcannon'
closed_at = <Date 2015-08-14.18:18:33.979>
created_at = <Date 2015-03-30.01:54:14.679>
labels = ['deferred-blocker', 'type-bug', 'library']
title = 'Suboptimal stacklevel of deprecation warnings for formatter and imp modules'
updated_at = <Date 2015-08-14.18:18:57.969>
user = 'https://bugs.python.org/Arfrever'

bugs.python.org fields:

activity = <Date 2015-08-14.18:18:57.969>
actor = 'brett.cannon'
assignee = 'brett.cannon'
closed = True
closed_date = <Date 2015-08-14.18:18:33.979>
closer = 'brett.cannon'
components = ['Library (Lib)']
creation = <Date 2015-03-30.01:54:14.679>
creator = 'Arfrever'
dependencies = []
files = ['39196', '39550']
hgrepos = []
issue_num = 23810
keywords = ['patch']
message_count = 11.0
messages = ['239554', '239607', '239610', '241944', '241968', '242015', '244260', '244304', '244396', '246285', '248600']
nosy_count = 7.0
nosy_names = ['brett.cannon', 'ncoghlan', 'larry', 'Arfrever', 'eric.snow', 'berker.peksag', 'serhiy.storchaka']
pr_nums = []
priority = 'deferred blocker'
resolution = 'duplicate'
stage = 'patch review'
status = 'closed'
superseder = '24305'
type = 'behavior'
url = 'https://bugs.python.org/issue23810'
versions = ['Python 3.5', 'Python 3.6']

[Arfrever]

https://hg.python.org/cpython/rev/2a336cc29282 changed stacklevel of some deprecation warnings.
However new value is still not useful, because either _frozen_importlib or importlib/_bootstrap.py is now mentioned in deprecation warnings:

$ cat test.py
import formatter
import imp
$ python3.4 -Wd test.py
/usr/lib64/python3.4/formatter.py:24: PendingDeprecationWarning: the formatter module is deprecated and will be removed in Python 3.6
  'Python 3.6', PendingDeprecationWarning)
/usr/lib64/python3.4/imp.py:32: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  PendingDeprecationWarning)
$ python3.5 -Wd test.py
_frozen_importlib:321: DeprecationWarning: the formatter module is deprecated and will be removed in Python 3.6
/usr/lib64/python3.5/importlib/_bootstrap.py:321: PendingDeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
  return f(*args, **kwds)

[brettcannon]

Probably need to introduce a new keyword argument just for deprecated imports or some helper function in importlib to get the stack depth right (else there is a risk of breaking the stack depth in any minor release whenever importlib's depth shifts). Something like the following should be enough (obviously done in warnings instead of per-file):

try:
level = 1
while True:
frame = sys._getframe(level)
print(frame.f_code.co_filename)
if '_bootstrap' not in frame.f_code.co_filename:
break
level += 1
except ValueError:
pass
print(sys._getframe(2).f_code.co_filename)
warnings.warn("the imp module is deprecated in favour of importlib; "
"see the module's documentation for alternative uses",
PendingDeprecationWarning, stacklevel=level+1)

Otherwise the depths should just go back to what they were at.

[serhiy-storchaka]

Similar feature is needed for warnings in the re module. Methods of regular expression pattern can be called directly or from module-level wrappers. In these case the stack level differs by 1. And sometimes warnings are emitted in recursive parser, when stack level is variable.

[brettcannon]

Here is a private function in warnings for calculating the stack depth to the first frame not referencing some key string. Can someone look at it to make sure it looks reasonable?

I'm starting with it being private since it uses sys._getframe() and I don't know how widely needed it is.

[serhiy-storchaka]

Unfortunately this will not help for re, because the trace passes through three files: Lib/sre_parse.py, Lib/sre_compile.py and Lib/re.py.

[brettcannon]

OK, I'll look at making it more general for multiple names to skip.

[ncoghlan]

bpo-24305 covers either making this a public API for general use, or else just making module level deprecation warnings skip the import machinery automatically.

I also wonder whether Eric's _boostrap_external changes might have broken any of the frame hiding tricks for tracebacks.

[ericsnowcurrently]

I had a similar concern, Nick, but don't think I did anything that would have broken the frame hiding logic. That said, I did not take stacklevel for warnings into account.

[brettcannon]

Latest patch should work for Serhiy's needs by taking a container of names to compare against the filename instead of a single argument.

[larryhastings]

This regression isn't thrilling, but it's not the kind of "OMG we can't release with this bug" level of escalation I associate with an actual release blocker. Let's at least defer it for now, and maybe we'll even reduce it further later.