Mailing List Archive: Python: Bugs

[issue2772] Add PendingDeprecationWarning for % formatting

Index | Next | Previous | View Threaded

report at bugs

May 6, 2008, 7:14 AM

Post #1 of 12 (1981 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting

New submission from Eric Smith <eric[at]trueblade.com>:

Per http://mail.python.org/pipermail/python-3000/2008-April/013094.html,
add a PendingDeprecationWarning to 3.0 for % formatting.

The attached patch implements this for 3.0. For 2.6, it should only be
a PendingDeprecationWarning if -3 warnings are turned on. I'll work on
that after the 3.0 patch is accepted.

I'm not wild about using a global variable to disallow recursion, but
it's probably the way to go. Question: Do I need to acquire the GIL here?

Another issue is that the interpreter should probably at least start up
without triggering these warnings. I'll add an issue for that at a
later date.

----------
assignee: eric.smith
components: Interpreter Core
files: percent_formatting_pending_deprecation.diff
keywords: easy, patch, patch
messages: 66313
nosy: eric.smith
priority: normal
severity: normal
status: open
title: Add PendingDeprecationWarning for % formatting
versions: Python 2.6, Python 3.0
Added file: http://bugs.python.org/file10202/percent_formatting_pending_deprecation.diff

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 7:48 AM

Post #2 of 12 (1927 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Amaury Forgeot d'Arc <amauryfa[at]gmail.com> added the comment:

> Do I need to acquire the GIL here?
No, the GIL has already been acquired.

The problem with the static variable is that while the main thread is
PyErr_WarnEx(), another thread may gain focus. And it will not trigger
the warning.

----------
nosy: +amaury.forgeotdarc

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 7:55 AM

Post #3 of 12 (1931 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Eric Smith <eric[at]trueblade.com> added the comment:

Right. But is it worth adding per-thread machinery to this? I was
going to do that, but it seemed like overkill. Upon further reflection,
however, I think you may be right.

I'll remove the "easy" keyword!

----------
keywords: -easy

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 2:16 PM

Post #4 of 12 (1916 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Benjamin Peterson <musiccomposition[at]gmail.com> added the comment:

Would it be practical to add a _PyErr_InErrorProcessing function to
prevent recursion?

----------
nosy: +benjamin.peterson

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 2:50 PM

Post #5 of 12 (1917 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Eric Smith <eric[at]trueblade.com> added the comment:

Since we're just trying to prevent this function from recursing
(indirectly) into itself, I think all of the logic can go here.

What would you suggest the function _PyErr_InErrorProcessing do differently?

I think the real issue is: Does the additional logic and execution time
involved in adding per-thread state justify being "more correct", or can
we occasionally lose a warning message?

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 2:58 PM

Post #6 of 12 (1919 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Benjamin Peterson <musiccomposition[at]gmail.com> added the comment:

On Tue, May 6, 2008 at 4:50 PM, Eric Smith <report[at]bugs.python.org> wrote:
>
> Eric Smith <eric[at]trueblade.com> added the comment:
>
> Since we're just trying to prevent this function from recursing
> (indirectly) into itself, I think all of the logic can go here.
>
> What would you suggest the function _PyErr_InErrorProcessing do differently?

I was just thinking that this problem would probably come up again.

> I think the real issue is: Does the additional logic and execution time
> involved in adding per-thread state justify being "more correct", or can
> we occasionally lose a warning message?

Well, the first thing to check for is Py_Py3kWarning. Then do the
extra logic and execution speed.

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 4:36 PM

Post #7 of 12 (1915 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Eric Smith <eric[at]trueblade.com> added the comment:

> Well, the first thing to check for is Py_Py3kWarning. Then do the
> extra logic and execution speed.

In 3.0, it's always a PendingDeprecationWarning, so that won't work.
The test needs to be:

if not recursing and warning_is_not_suppressed:
warn()

The recursion test is expensive if using thread local storage; the
warning suppressed test looks expensive, too. So there's no quick short
circuit test that I see. Of course all of this is just hot air until
coded and benchmarked. I'll cook up a patch, but it will probably not
be ready before the next alpha releases.

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 6, 2008, 9:03 PM

Post #8 of 12 (1912 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Neal Norwitz <nnorwitz[at]gmail.com> added the comment:

Why not use the normal recursion check mechanism? Specifically,

if (Py_EnterRecursiveCall("unicode % "))
return NULL;
// err = Warn();
Py_LeaveRecursiveCall();

I don't see where the problem with threads comes in. The GIL is held
and shouldn't be released during this call. That may not be quite true
(it's conceivable the GIL is released when warning). I'm not sure what
happens with the I/O system at this point, it's possible that releases
the GIL. However, if GIL is released and re-acquired in PyWarn_WarnEx()
there are probably bigger issues than this patch that will need to be
addressed. Note that since the warnings module is now implemented in C,
this should be easier to deal with.

Using the macros above in essence uses TLS, but through Python's
PyThreadState.

----------
nosy: +nnorwitz

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 7, 2008, 8:33 AM

Post #9 of 12 (1911 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Eric Smith <eric[at]trueblade.com> added the comment:

I'm not sure Py_EnterRecursiveCall is what I want, because that allows
the recursion to happen, but just aborts it when it gets too deep. What
I want to achieve is to have the warning not called if it's the warning
that's being formatted. I coded this up and couldn't get it to do the
right thing.

I think a better approach will be to remove % formatting from
warnings.py. I think that will remove the need for any checks at all.
I'll investigate that.

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 7, 2008, 11:49 AM

Post #10 of 12 (1905 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Benjamin Peterson <musiccomposition[at]gmail.com> added the comment:

On Wed, May 7, 2008 at 10:33 AM, Eric Smith <report[at]bugs.python.org> wrote:
> I think a better approach will be to remove % formatting from
> warnings.py. I think that will remove the need for any checks at all.
> I'll investigate that.

That would make user code warning that uses '%"' brittle. However, if
we warn about it, I think it's ok.

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 7, 2008, 6:26 PM

Post #11 of 12 (1896 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Eric Smith <eric[at]trueblade.com> added the comment:

> That would make user code warning that uses '%"' brittle. However, if
> we warn about it, I think it's ok.

True enough. Then I think we should go with:
1. Use .format() in the warnings module.
2. Tell the users not to use % formatting in user code for warnings.
3. Add my original, simple, global check for recursion. It will work
incorrectly in an insignificant number of cases, and will typically
result in at least one warning about % formatting, anyway.

I'll have a patch ready soon.

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

report at bugs

May 9, 2008, 5:58 AM

Post #12 of 12 (1873 views)
Permalink

[issue2772] Add PendingDeprecationWarning for % formatting [In reply to]

Eric Smith <eric[at]trueblade.com> added the comment:

The attached patch (percent_formatting_pending_deprecation_0.diff) adds
both the simple global lock to unicode_mod() and changes warnings.py to
use .format() instead of % formatting.

I think this is good enough. We should add a warning to the docs saying
user code used for warnings should also avoid % formatting. The only
problem they'll see, however, is dropping an occasional warning in a
multi-threaded app. But I'm not sure yet exactly where this mention
would go.

I also don't have any tests for this. I haven't found a test of a
PendingDeprecationWarning.

The biggest problem is that this patch breaks test_doctest with
"RuntimeError: dictionary changed size during iteration". I've tried to
find out why, but so far it escapes me. If someone else could look at
the issue, I'd appreciate it.

Added file: http://bugs.python.org/file10232/percent_formatting_pending_deprecation_0.diff

__________________________________
Tracker <report[at]bugs.python.org>
<http://bugs.python.org/issue2772>
__________________________________
_______________________________________________
Python-bugs-list mailing list
Unsubscribe: http://mail.python.org/mailman/options/python-bugs-list/list-python-bugs%40lists.gossamer-threads.com

Index | Next | Previous | View Threaded

Interested in having your list archived? Contact lists@gossamer-threads.com