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.

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.

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!

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

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?

On Tue, May 6, 2008 at 4:50 PM, Eric Smith <report@bugs.python.org> wrote:

Eric Smith <eric@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.

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.

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.

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.

On Wed, May 7, 2008 at 10:33 AM, Eric Smith <report@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.

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.

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.

I think this is premature until be know for sure that % formatting will
in-fact be deprecated in Py3.1. Time will tell how well the new format
options get accepted. Likewise, we'll learn more about how readily
legacy code can get converted.

Guido, can you pronouce?

I agree. That's one of the reasons I un-assigned it to me.

Well, that and I couldn't get it to pass all tests.

Not it.

Am closing this one. It is pointless and counter-productive unless
there is a firm decision that % formatting is definitely going away (and
migration tools have been developed). Right now, it looks like there is
some chance that it might pull through for a few versions.