I'm using Python 3.1.2 (r312:79149, Mar 21 2010, 00:41:52) [MSC v.1500 32 bit (Intel)] on win32. Running the following code:


    import profile
    import math
    import decimal
    
    
    def show_bug():
        x = decimal.Decimal.from_float(math.e)**(-2**31)
    
    profile.run('show_bug()')

And I get this stack trace:

Traceback (most recent call last):
  File "C:/Users/Me/Documents/python/temptest", line 15, in <module>
    profile.run('show_bug()')
  File "C:\Python31\lib\profile.py", line 70, in run
    prof = prof.run(statement)
  File "C:\Python31\lib\profile.py", line 456, in run
    return self.runctx(cmd, dict, dict)
  File "C:\Python31\lib\profile.py", line 462, in runctx
    exec(cmd, globals, locals)
  File "<string>", line 1, in <module>
  File "C:/Users/Me/Documents/python/temptest", line 8, in show_bug
    x = Decimal.from_float(math.e)**(-2**31)
  File "C:\Python31\lib\decimal.py", line 2212, in __pow__
    context = getcontext()
  File "C:\Python31\lib\decimal.py", line 447, in getcontext
    context = Context()
  File "C:\Python31\lib\decimal.py", line 3777, in __init__
    for name, val in locals().items():
RuntimeError: dictionary changed size during iteration

I can't reproduce this on Linux.  However, I've seen an informal report of something like this happening before.  It looks a lot like something threading related, but I don't see any threads in what you're doing.  (Though I don't know the details of how the profile module works, on either Linux or Windows.)

The use of locals() in the Context constructor is an unnecessary hack that's easy to fix;  I'll do that.  But it would still be good to understand exactly where this is coming from.

BTW, is the behaviour consistent, or does it only occur on some runs?

Okay, I can reproduce by adding a 'time.sleep(0.01)' delay into the body of the 'for name, val in locals().items():' loop.  I then get (with py3k):

dickinsm@alberti:~/Source/py3k> ./python
Python 3.2a0 (py3k:82413M, Jul  1 2010, 10:21:02)
[GCC 4.2.1 (SUSE Linux)] on linux2
Type "help", "copyright", "credits" or "license" for more information.
>>> import decimal, profile
>>> def show_bug(): decimal.Decimal(1.2)**(-2**31)
...
>>> profile.run('show_bug()')
Traceback (most recent call last):
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 447, in getcontext
    return _local.__decimal_context__
AttributeError: '_thread._local' object has no attribute '__decimal_context__'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/dickinsm/Source/py3k/Lib/profile.py", line 70, in run
    prof = prof.run(statement)
  File "/home/dickinsm/Source/py3k/Lib/profile.py", line 442, in run
    return self.runctx(cmd, dict, dict)
  File "/home/dickinsm/Source/py3k/Lib/profile.py", line 448, in runctx
    exec(cmd, globals, locals)
  File "<string>", line 1, in <module>
  File "<stdin>", line 1, in show_bug
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 2222, in __pow__
    context = getcontext()
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 449, in getcontext
    context = Context()
  File "/home/dickinsm/Source/py3k/Lib/decimal.py", line 3822, in __init__
    for name, val in locals().items():
RuntimeError: dictionary changed size during iteration

Mark Dickinson <report@bugs.python.org> wrote:
> I can't reproduce this on Linux.  However, I've seen an informal report of something like this happening before.  It looks a lot like something threading related, but I don't see any threads in what you're doing.  (Though I don't know the details of how the profile module works, on either Linux or Windows.)

I can reproduce it consistently, also --without-threads. I'm lacking knowledge
about the profile module, but it has a comment "(old profiler used to write into
the frames local dictionary!!". Perhaps there's still something like that going on.

Removing the decimal module from the equation, the following is enough to trigger this for me.  Stefan's suggestion about the profile module writing to locals sounds right on target.


import profile

class Context(object):
    def __init__(self):
        for name, val in locals().items():
            setattr(self, name, val)

profile.run("Context()")

class Context(object):
    def __init__(self):
        for name, val in locals().items():
            setattr(self, name, val)


Isn't this dodgy anyway, since 'name' and 'val' end up going into locals()?  I wonder why the RuntimeError *isn't* raised for a normal 'Context()' call (i.e., not via profile).

Still clueless about profile.py, but I think it creates a parallel stack,
and overwriting of locals legitimately occurs in that stack, producing
the Exception.

So it would appear that by design one simply cannot iterate over locals
when using the profile module.

Mark Dickinson <report@bugs.python.org> wrote:
> 
> Mark Dickinson <dickinsm@gmail.com> added the comment:
> 
> class Context(object):
>     def __init__(self):
>         for name, val in locals().items():
>             setattr(self, name, val)
> 
> 
> Isn't this dodgy anyway, since 'name' and 'val' end up going into locals()?  I wonder why the RuntimeError *isn't* raised for a normal 'Context()' call (i.e., not via profile).

Indeed, and it seems to depend on how name and vals are used:

...     print(locals())
... 
{'name': '__builtins__', 'val': <module 'builtins' (built-in)>, '__builtins__': <module 'builtins' (built-in)>, '__package__': None, 'Context': <class '__main__.Context'>, '__name__': '__main__', '__doc__': None}
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: dictionary changed size during iteration

...     pass
... 
>>> locals()
{'name': '__doc__', 'val': None, '__builtins__': <module 'builtins' (built-in)>, '__package__': None, 'Context': <class '__main__.Context'>, '__name__': '__main__', '__doc__': None}
>>>

The tracker doesn't handle code when posted by mail. Here's the
code again:

>>> for name, val in locals().items():
...     print(locals())
... 
{'name': '__builtins__', 'val': <module 'builtins' (built-in)>, '__builtins__': <module 'builtins' (built-in)>, '__package__': None, '__name__': '__main__', '__doc__': None}
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
RuntimeError: dictionary changed size during iteration

 
>>> for name, val in locals().items():
...     pass
... 
>>> 
>>> locals()
{'name': '__doc__', 'val': None, '__builtins__': <module 'builtins' (built-in)>, '__package__': None, '__name__': '__main__', '__doc__': None}
>>>

Ah, it looks like 'locals()' is somewhat magical.  Its docstring says:

"Update and return a dictionary containing the current scope's local variables."

So I think this explains your (Stefan's) results:  in either case, you evaluate locals() (as the target of the for statement) and get a dictionary back.  But that dictionary isn't updated to include 'name' and 'val' until you call locals() for a second time.  (And possibly there are other activities besides an explicit locals() call that would cause that dict to be updated, but I'm not sure.)

I still don't understand how things work when profile is added into the mix, but I'm willing to accept that the profile module affects locals() in strange and possibly timing-dependent ways.

Anyway, the fix for decimal is clear:  get rid of that locals call.

> it looks like 'locals()' is somewhat magical

... and Objects/frameobject.c is the place to go for a full understanding.  PyFrame_FastToLocals is the 'updating' function that updates the locals dict from the 'fast locals' object.

Here's a patch.  It's a little ugly, but I don't see a better way that avoids locals().  Using **kwargs is an option, but would mean abandoning the current nice feature that unexpected keyword arguments raise TypeError (or alternatively, implementing that check separately).

This should be backported to 2.x after 2.7 has been released;  for that backport, the dictionary comprehensions should be replaced by dict(<list_comprehension>), so that the code remains valid with older Python versions.

Mark, the patch looks good. I don't find it ugly, but anyway, here's a
bike shed version. :)

The main point is that I'd like the flags and traps sections to be
set apart visually while compressing the boilerplate.

Nice shade of blue!  Just a couple of red spots that I'd prefer repainted, namely: (1) please put bodies of 'try:' and 'except:' on separate lines, and (2) please use 'except NameError' instead of a bare except.

And a couple of points that I think also applied to my patch:

- {s: 0 for s in _signals} could also be spelt dict.fromkeys(_signals, 0)

- I think dict(dc.traps) should be dc.traps.copy() instead.  I can't entirely see why it would matter, except that it's conceivable that dc.traps could by a dict subclass with its own copy method.

Stefan, one other thought: please also bear in mind that we're restricted to Python 2.3 syntax in the 2.x version of decimal, so any post-Python 2.3-isms will have to be rewritten when the patch is backported.  (E.g., use of conditional expressions.)

While I don't see any particular reason for the 3.x version of decimal to restrict its syntax in this manner, it might be worth sticking to simple syntax just so that the 2.x and 3.x decimal versions don't differ too much, to make maintenance easier.

Mark, good point about 2.3 compatibility. The unit tests diverge quite a
bit though between 2.5, 2.6 and 2.7.

I like {s: 0 for s in _signals} slightly better here (but I like
dict/list comprehensions in general).


So, the new patch still sets the flags/traps sections apart, doesn't
use the ternary construct and uses traps.copy().


IOW, I only painted the door blue. ;)

Looks good to me.  Please go ahead and apply it to the 3.2 and 3.1 branches, if you want.  (Or just assign back to me if you prefer.)

It should also be applied to 2.7 and 2.6, eventually, but we should probably wait until after the 2.7 release this weekend for that.

Stylistically, it would be nice to eliminate the local variable reassignment entirely:

self.Emin = DefaultContext.Emin if Emin is None else Emin
self.Emax = DefaultContext.Emax if Emax is None else Emax
self._ignored_flags = [] if _ignored_flags is None else _ignored_flags

Also, to keep the code consistent between versions, it may be better to avoid set/dict comprehensions and stick with the old:

    dict([(s, int(s in flags)) for s in _signals])

or slightly more modern generator comprehension:

    dict((s, int(s in flags)) for s in _signals)

[Raymond]
> self.Emin = DefaultContext.Emin if Emin is None else Emin

I agree that looks better.  But we can't spell it that way in 2.x (since conditional expressions aren't 2.3 compatible), and I'd prefer to keep the 2.x and 3.x versions reasonably close, at least while 2.x is still being maintained.

Still, no strong feelings either way.

I'm not sure that I care about 2.3 compatibility anymore (I was the one who made the rule about 2.3 compatibility and made sure that it was just a preference, not an absolute rule -- as time goes on, it is of less and less value).  ISTM, 2.5 compatible is probably good enough.

> ISTM, 2.5 compatible is probably good enough.

Okay;  that's fine with me.  Now we can finally turn that "from_float = classmethod(from_float)" into a proper "@classmethod" decorator.  :)

I also don't think that the 2.x-to-3.x maintenance issue is that big a deal any more;  it would be surprising if the 2.x version of Decimal sees anything more than minor changes (doc fixes, etc.) from this point on.  So perhaps the 3.x version of the decimal module should be free to make full use of 3.x syntax?

Now that we're on the subject of "from_float", I just recalled this slight
issue:

Python 3.1.2 (r312:79149, Mar 21 2010, 00:41:52) [MSC v.1500 32 bit (Intel)]
on
win32
Type "help", "copyright", "credits" or "license" for more information.
>>>
>>> import decimal
>>> x = decimal.Decimal()
>>> decimal.Decimal.from_float(x)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "C:\Python31\lib\decimal.py", line 687, in from_float
    n, d = abs(f).as_integer_ratio()
AttributeError: 'Decimal' object has no attribute 'as_integer_ratio'
>>>

It seems from_float doesn't like it when a Decimal arrives. Personally, I
think there should be an idiomatic way of saying "this number must be a
Decimal" without an "isinstance".

Should I open another ticket?

> I also don't think that the 2.x-to-3.x maintenance issue 
> is that big a deal any more;  it would be surprising if 
> the 2.x version of Decimal sees anything more than minor 
> changes (doc fixes, etc.) from this point on.  So perhaps 
> the 3.x version of the decimal module should be free to 
> make full use of 3.x syntax?

I would like to still leave us at 2.5 compatible.
The internal comments promise that spec updates
will be treated as bug fixes.  This is all the
more likely if 2.7 is the last, but long lived
version in the 2.x series.

Besides, I don't think the set comprehension
notation was a big win in readability over
what is there now :-)

2.5 compatibility is fine for the 2.x version of decimal.py.

For the 3.x version of decimal.py, I don't see what it buys us.

In the new patches I reinstated the ternary construct. I think it
would be nice to use dict comprehensions in py3k. People read the
stdlib for guidance, and it would help if ultimately only the most
concise idioms remained in py3k.

If I'm not mistaken, quite often the unit tests already use the latest
constructs available.


That said, there are always issues that only become apparent after
maintaining a module for years, so I guess I'll wait for a decision.

+        try:
+            dc = DefaultContext
+        except NameError:
+            pass
+
+        self.prec = dc.prec if prec is None else prec

I don't quite understand the point of catching NameError here, but it looks like if it is caught, it will just be raised by the next line because dc will not be set.

Also, I am not sure ternary expressions are a big readability win here, but if you want to keep them, consider 

self.prec = prec if prec is not None else dc.prec

> I don't quite understand the point of catching NameError here

So that the initialization of DefaultContext itself doesn't fail.

Alexander, for me the ternary expressions have the advantage that they
take up less vertical space and you have to keep less state in mind.


Mark, I'm sidestepping the dict syntax question and reassign to you :)

Fixed in revisions r82654 through r82657.  Thanks everyone for the input.