#7556 NORM 9.1.0: sugar.graphics.xocolor.py should not slay PEP 008 and dance on its grave

[Zarro Boogs per Child]

#7556: sugar.graphics.xocolor.py should not slay PEP 008 and dance on its grave
--------------------------+-------------------------------------------------
 Reporter:  mtd           |         Owner:  mtd               
     Type:  enhancement   |        Status:  new               
 Priority:  normal        |     Milestone:  9.1.0             
Component:  not assigned  |       Version:  Git as of bug date
 Keywords:  9.1.0:+ r?    |   Next_action:  review            
 Verified:  0             |     Blockedby:                    
 Blocking:                |  
--------------------------+-------------------------------------------------
 The diff speaks for itself, I think:

 {{{
 diff --git a/src/sugar/graphics/xocolor.py b/src/sugar/graphics/xocolor.py
 index d5e906f..72ae11e 100644
 --- a/src/sugar/graphics/xocolor.py
 +++ b/src/sugar/graphics/xocolor.py
 @@ -18,216 +18,223 @@
  import random

  colors = [
 -['#B20008', '#FF2B34'], \
 ...
 +    ['#BCCDFF', '#AC32FF'],
 +    ]

  def _parse_string(color_string):
      if color_string == 'white':
 -        return ['#ffffff', '#414141']
 +        return ('#ffffff', '#414141')
      elif color_string == 'insensitive':
 -        return ['#ffffff', '#e2e2e2']
 +        return ('#ffffff', '#e2e2e2')

 -    splitted = color_string.split(',')
 -    if len(splitted) == 2:
 -        return [splitted[0], splitted[1]]
 -    else:
 -        return None
 +    def _is_color_str(s):
 +        return len(s) == 7 and s.startswith("#") #better than previous
 test
 +
 +    parts = color_string.split(',')
 +    if len(parts) == 2 and _is_color_str(parts[0]) and
 _is_color_str(parts[1]):
 +        return parts[0], parts[1]
 +    return None

  def is_valid(color_string):
 -    return (_parse_string(color_string) != None)
 +    return _parse_string(color_string) is not None
 +

  class XoColor:
      def __init__(self, color_string=None):
 -        if color_string == None or not is_valid(color_string):
 -            n = int(random.random() * (len(colors) - 1))
 -            [self.stroke, self.fill] = colors[n]
 +        if color_string is None or not is_valid(color_string):
 +            self.stroke, self.fill = random.choice(colors)
          else:
 -            [self.stroke, self.fill] = _parse_string(color_string)
 +            self.stroke, self.fill = _parse_string(color_string)

      def __cmp__(self, other):
 -        if isinstance(other, XoColor):
 -            if self.stroke == other.stroke and self.fill == other.fill:
 -                return 0
 -        return -1
 +        if isinstance(other, XoColor) \
 +                and self.stroke == other.stroke \
 +                and self.fill == other.fill:
 +            return 0
 +        else:
 +            return -1
 +
 +    def __str__(self):
 +        return '%s,%s' % (self.stroke, self.fill)

      def get_stroke_color(self):
          return self.stroke
 @@ -236,7 +243,9 @@ class XoColor:
          return self.fill

      def to_string(self):
 -        return '%s,%s' % (self.stroke, self.fill)
 +        """deprecated -- use str(<instance>)"""
 +        return str(self)
 +

  if __name__ == "__main__":
      import sys
 }}}

-- 
Ticket URL: <http://dev.laptop.org/ticket/7556>
One Laptop Per Child <http://laptop.org/>
OLPC bug tracking system