search

Averroes / simplejson

MIT License
0 stars 0 forks source link

2.4 compatibility error #53

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Using Python 2.4.3.

Line 55 of ordered_dict.py is causing a problem.

Below is the diff of the change I made locally:

--- ../dev/simplejson/trunk/simplejson/ordered_dict.py  (revision 184)
+++ ../dev/simplejson/trunk/simplejson/ordered_dict.py  (working copy)
@@ -52,7 +52,7 @@
     def popitem(self, last=True):
         if not self:
             raise KeyError('dictionary is empty')
-        key = reversed(self).next() if last else iter(self).next()
+        key = last and reversed(self).next() or iter(self).next()
         value = self.pop(key)
         return key, value

Original issue reported on code.google.com by tzell...@gmail.com on 15 May 2009 at 1:04

GoogleCodeExporter commented 9 years ago 
I don't think that's a correct patch, if reversed(self).next() is None then 
iter(self).next() will be called.

Original comment by bob%redi...@gtempaccount.com on 15 May 2009 at 3:38

GoogleCodeExporter commented 9 years ago 
You are correct - my bad on that.

I guess my intent was to point out that simplejson is using the conditional
expressions introduced in Python 2.5 (PEP 308), but otherwise is 2.4-compatible.

A better fix would be to expand the conditional statement (I know, it would 
lose in
golf)...

if last:
    key = reversed(self).next()
else:
    key = iter(self).next()

Original comment by tzell...@gmail.com on 15 May 2009 at 4:47

GoogleCodeExporter commented 9 years ago 
That patch looks better. Note that all *released* versions are still compatible 
with 2.4. I'll definitely take this into 
consideration and either stop supporting 2.4 for simplejson 2.1+ or make sure 
that OrderedDict gets properly 
backported. It looks like your fix probably would work, but there could be 
other subtleties with 2.4.

Original comment by bob%redi...@gtempaccount.com on 15 May 2009 at 5:04

GoogleCodeExporter commented 9 years ago 
also needed to implement all(), r186 r189

Original comment by bob.ippo...@gmail.com on 17 May 2009 at 6:53