[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Larry Hastings added the comment: If this has been broken since 3.3, I don't think it's a release blocker for 3.5. I'm willing to consider it a bug and accept a fix, but I'd prefer it to be as low-risk as possible (aka the Python version). Can someone fix the regressions? And, if the C fix is better somehow, let's definitely get that into 3.6. -- priority: release blocker - deferred blocker ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Larry Hastings added the comment: The C implementation is making me nervous. My gut feeling is the Python implementation would be easier to get right. I still don't quite understand: what is the user-perceived result of this change? Module authors issuing a DeprecationWarning can now use stacklevel=2 instead of stacklevel=10? -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nathaniel Smith added the comment: I still don't quite understand: what is the user-perceived result of this change? Module authors issuing a DeprecationWarning can now use stacklevel=2 instead of stacklevel=10? Exactly. There are a lot of deprecated modules in the wild, and the correct way to mark a module as deprecated is by writing warnings.warn(This module is deprecated!, stacklevel=2) at the top-level of your module. Except that in Python 3.3 you have to use stacklevel=10, and in Python 3.4 you have to use stacklevel=8, and I haven't checked what you need in Python 3.5 but it may well be different again, because it depends on internal implementation details of the import system. Fortunately AFAICT from some code searches no-one is actually depending on this though; instead everyone just writes stacklevel=2 and gets the wrong result. (This is made more urgent b/c people are increasingly relying on the stacklevel information to filter whether they even see the DeprecationWarning. E.g. I've seen multiple test suites which register a warnings filter that displays DeprecationWarnings iff they are attributed to the package-under-test, and IPython now shows deprecation warnings by default if the warning is attributed to interactive code -- see issue 24294. So there's a good chance that users will only find out that the module they are using is deprecated if the stacklevel= is set correctly.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nathaniel Smith added the comment: You're right, impossible is a slight exaggeration :-). As an alternative, every package could indeed carry around a table containing the details of importlib's call stack in every version of Python. (I also haven't checked whether it's consistent within a single stable release series. I guess we could add a rule that 3.5.x - 3.5.(x+1) cannot change the number of function calls inside the importlib callstack, because that is part of the public API, but I have less than perfect confidence that this rule has been enforced strictly in the past, and I don't think I'd want to be the one in charge of enforcing it in the future.) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Larry Hastings added the comment:
Is it really *impossible* to correctly issue a deprecation warning for a
module, as the title asserts? Or does the new import system simply make it
*tiresome*?
if sys.version_info.major == 3 and sys.version_info.minor == 4:
stacklevel = 8
elif sys.version_info.major == 3 and sys.version_info.minor == 4:
stacklevel = 10
else:
stacklevel = 2 # I bet they fixed it in 3.6!
warnings.warn({} is deprecated.format(__name__), DeprecationWarning,
stacklevel=stacklevel)
That's Python for you, doing six impossible things before breakfast.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24305
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Here is a patch that adds the C version of the frame skipping. Unfortunately it fails under test_threading, test_subprocess, test_multiprocessing_spawn. It's due to is_internal_frame() somehow although setting a breakpoint in gdb in that function never triggers. The return value is -11 from the interpreter and I don't remember what that represents. -- Added file: http://bugs.python.org/file40233/issue24305.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Josh Rosenberg shadowranger+pyt...@gmail.com: -- nosy: +josh.r ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Eric Snow added the comment: Hybrid Nathaniel/Brett approach LGTM -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Could someone do a quick code review of my latest patch? If the design looks sane I might be able to steal some time tomorrow to start on the C implementation. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Here is an approach that somewhat merges Nathaniel's proposed solution and mine. -- Added file: http://bugs.python.org/file40211/issue24305.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Here is a new version of the patch with Nathaniel's and and Berker's comments addressed. -- Added file: http://bugs.python.org/file40199/issue24305.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Here is a pure Python patch that skips any frames that mentions 'importlib' and '_bootstrap' in the filename (it also skips _warnings.warn for easy testing since I didn't implement the C part). The only side-effect is that negative stack levels won't be the same because of the change in line numbers, but in that instance I'm not going to worry about it. It also causes test_venv and test_warnings to fail but I have not looked into why. And still need a call by Larry as to whether this will go into 3.5.0. -- components: +Library (Lib) keywords: +patch stage: - patch review Added file: http://bugs.python.org/file40187/issue24305.diff ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nathaniel Smith added the comment: Yeah, yours is probably better in fact, I was just trying to make the semantics as obvious as explicit as possible for a comment :-) -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Will that be thread-safe? Plus storing that value in a way that _warnings and warnings can read will require something like a single-item list that can be mutated in-place. The other option is to teach warnings to skip anything coming from importlib/_bootstrap* unless stacklevel==1 and that frame happens to be in the importlib code. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Serhiy Storchaka added the comment: Frames can be skipped only for warnings issued by imported module code, not for warnings issued by import machinery itself. I propose following algorithm: Before executing module code push the current frame and the number of frames to skip in global stack in the warnings module, and pop it after execution (public context manager can help). In warnings.warn() skip stacklevel frames and if saved frame was skipped, skip corresponding additional number of frames. Repeat for all saved frames. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nathaniel Smith added the comment: For 3.4/3.5 purposes, I propose a simpler algorithm: first, define a function which takes a module name and returns true if it is part of the internal warning machinery and false otherwise. This is easy because we know what import machinery we ship. Then, to walk the stack in warnings.py, do something like: frame = sys._get frame(1) if is_import_machinery(frame.module_name): skip_frame = lambda modname: False else: skip_frame = is_import_machinery def next_unskipped_frame(f): new = f while new is f or skip_frame(new.module_name): new = new.caller for i in range(stacklevel - 1): frame = next_unskipped_frame(frame) This produces reasonable behavior for warnings issued by both regular user code and by warnings issued from inside the warning machinery, and it avoids having to explicitly keep track of call depths. Then we can worry about coming up with an all-singing all-dancing generalized version for 3.6. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Brett Cannon br...@python.org: -- assignee: - larry nosy: +larry ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: I have merged over issue #23810 because they are aiming to solve the same problem and the conversation is split too much. Just thinking out loud, this situation is compounded by the fact that importlib itself has some warnings and so automatically stripping out stack frames might make those warnings look odd. Still not sure if Larry considers this a release blocker for 3.5.0. And any solution will need to be solved in warnings.py and _warnings.c. Some days I really hate our warnings system. -- dependencies: -Suboptimal stacklevel of deprecation warnings for formatter and imp modules keywords: +3.2regression priority: normal - release blocker versions: +Python 3.5, Python 3.6 ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Nathaniel's solution is basically what I came up with in issue #23810 except I simply skipped subtracting from the stack level if it was found to be an internal import frame instead of swapping in and out a callable. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Jakub Wilk jw...@jwilk.net: -- nosy: +jwilk ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nathaniel Smith added the comment:
A nice new API is certainly a nice thing, but I feel that changing the stack
walking code should be considered as fixing a regression introduced in 3.3.
Indeed, searching github for code that actually uses the stacklevel= argument:
https://github.com/search?p=2q=warnings.warn+DeprecationWarning+stacklevelref=searchresultstype=Codeutf8=%E2%9C%93
I observe that:
- there isn't a single usage on the first ten pages with stacklevel 3, so
stack walking speed is unlikely to be an issue -- esp. since it will only be
slow in the cases where there are spurious import frames on the stack, i.e.,
you can only make it faster by making the results incorrect, and
- in the first ten pages I counted 14 distinct pieces of code (GH search is
chock full of duplicates, sigh), and *11* of them are specifically module
deprecations that are correctly passing stacklevel=2, and thus working on 2.7
but broken on 3.3+, and
- I counted zero examples where someone wrote
if sys.version_info[:2] == (3, 3):
stacklevel = 10
elif sys.version_info[:2] == (3, 4):
stacklevel = 8
else:
stacklevel = 2
warnings.warn({} is deprecated.format(__name__), DeprecationWarning,
stacklevel=stacklevel)
which is the only situation where even backporting a fix for this would break
code that previously worked correctly.
Basically the overwhelming majority of uses of the stacklevel= argument are
broken in 3.3 and 3.4 and 3.5-beta. We should fix it.
--
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24305
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: Skipping select frames is a shift in semantics for warnings.warn() (so can't go into 3.5b1), doing it implicitly might be costly on interpreters where getting a frame is expensive, and coming up with a new API allows for a nicer design, e.g. `warnings.deprecate_module(__name__, 'it will be removed in Python 3.6')` - the formatter module is deprecated; it will be removed in Python 3.6 -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com: -- nosy: +Arfrever ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Brett Cannon added the comment: My personal plan was to get issue #23810 finished, make sure it worked, and then expose a public API for declaring module deprecations which used the private API under the hood. I'm hoping to get #23810 done this Friday and then we can talk about how we may want to expose a function in the warnings module for deprecating modules. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Serhiy Storchaka added the comment: It would be better to skip _frozen_importlib frames automatically instead of forcing end users to use special API. -- nosy: +serhiy.storchaka ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Thomas Kluyver tak...@gmail.com: -- nosy: +takluyver ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nick Coghlan added the comment: Somewhat related issues are issue 16217 (traceback noise when running under python -m) and issue 23773 (traceback noise when running under custom import hooks) For 3.4 and 3.5, we may want to consider a brute force solution where the warnings module just straight up ignores _frozen_importlib frames when counting stack levels. For 3.6+, I've occasionally pondered various ideas for introducing the notion of a frame debugging level. The most recent variant of that idea would be to have all frames live in level zero by default, but there'd be a way to flag a module at compile time to set it higher for the code objects created by that module. Displaying those frames in tracebacks would then be tied to both the current frame's debugging level and the interpreter's verbosity level (whichever was higher), while other stack related operations (like the warnings stacklevel and frame retrieval) would only consider frames at the current frame's debugging level and below. -- ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Nick Coghlan added the comment: I've made this depend on issue 23810, rather than duplicating it. That way, issue 23810 can cover fixing the stdlib deprecation warnings, while this can cover making it a public API. -- dependencies: +Suboptimal stacklevel of deprecation warnings for formatter and imp modules ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Ned Deily n...@acm.org: -- nosy: +brett.cannon, eric.snow, ncoghlan ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
New submission from Nathaniel Smith:
(Noticed while fixing the IPython equivalent of issue 24294)
The obvious way to deprecate a module is to issue a DeprecationWarning inside
the main body of the module, i.e.
# thirdpartymodule.py
import warnings
warnings.warn({} is deprecated.format(__name__), DeprecationWarning)
# mymodule.py
import thirdpartymodule
But this is problematic, because the resulting message will claim that the
problem is in thirdpartymodule.py, not in mymodule.py. And this is especially
bad if I am doing things correctly (!) and using a warnings filter that enables
display of DeprecationWarnings for mymodule, but not for third-party modules.
(This need for correct attribution comes up in the interactive use case cited
above, but I actually have packages where the CI infrastructure requires the
elimination of DeprecationWarnings triggered by my own code -- for this to work
it's crucial that warnings be attributed correctly.)
So the obvious fix is to instead write:
# thirdpartymodule.py
import warnings
warnings.warn({} is deprecated.format(__name__), DeprecationWarning,
stacklevel=2)
which says the code that needs fixing is the code that called me.
On Python 2.7, this works, because all the code that executes in between
'import thirdpartymodule' and the call to 'warnings.warn' is C code, so it
doesn't create any intermediate stack frames.
On more recent versions of Python, the import system itself is written in
Python, so this doesn't work at all.
On Python 3.3, the correct way to deprecate a module is:
warnings.warn(this module is deprecated, DeprecationWarning,
stacklevel=10)
and on Python 3.4, the correct way to deprecate a module is:
warnings.warn(this module is deprecated, DeprecationWarning,
stacklevel=8)
(See https://github.com/ipython/ipython/pull/8480#issuecomment-106107638 for
test code.)
Obviously this is not desireable.
I'm not sure what best solution is. Maybe there should be some collaboration
between the import code and the warnings module, so that when the warnings
module walks the stack, it skips over stack frames that come from inside the
guts of import system?
--
messages: 244225
nosy: njs
priority: normal
severity: normal
status: open
title: The new import system makes it impossible to correctly issue a
deprecation warning for a module
type: behavior
___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue24305
___
___
Python-bugs-list mailing list
Unsubscribe:
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Changes by Barry A. Warsaw ba...@python.org: -- nosy: +barry ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
[issue24305] The new import system makes it impossible to correctly issue a deprecation warning for a module
Berker Peksag added the comment: This looks like a duplicate of issue 23810 (there is a patch for stdlib usages, but it probably can be changed to a more general solution). -- nosy: +berker.peksag ___ Python tracker rep...@bugs.python.org http://bugs.python.org/issue24305 ___ ___ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
