Opened 8 years ago

Last modified 7 years ago

#560 new defect

Pylint sugar

Reported by: marco Owned by: erikos
Priority: high Milestone: Opportunity
Component: sugar Version:
Keywords: Cc: mstone, tomeu, marco, erikos
Blocked By: Blocking:
Deployments affected: Action Needed:
Verified: no

Description (last modified by marco)

Here is I would suggest to attack it:

1)

  • Start from /usr/share/doc/pylint-0.12.1/examples/pylintrc, relax it a bit. As a first pass we only want to get problems like unused/undefines variables, code bugs etc.
  • Setup a script inside the sugar modules that devel can use to run it. It should just work, i.e. it should not require to create your own ~/.pylintrc

2)

  • Fix all the errors reported by pylint

3)

  • Make the pylint more strict, matching our code style guidelines.
  • Fix all the errors reported

At each step we should commit the code to the repo.

Attachments (1)

patch (1.5 KB) - added by jennjacobsen 7 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 8 years ago by marco

  • Description modified (diff)

comment:2 Changed 8 years ago by marco

  • Priority changed from normal to high

comment:3 Changed 8 years ago by marco

  • Milestone changed from BTest-2 to Opportunity

comment:4 Changed 8 years ago by marco

As a start I checked in a pylint.sh. All the errors we should fix are in the TODO variable in the script. If you want to help fixing one or more of these:

  • Remove one or more of them from the TODO variable
  • Run pylint.sh
  • Fix all the reported errors
  • Send a patch :)

Changed 7 years ago by jennjacobsen

comment:5 in reply to: ↑ description Changed 7 years ago by jennjacobsen

  • Keywords review? added; sugar-love removed

I started pylint and found a variable used without assignment error. Benzea suggested the fix, patch attached.

comment:6 Changed 7 years ago by marco

  • Keywords review? removed

Pushed, thanks!

comment:7 in reply to: ↑ description Changed 7 years ago by jennjacobsen

  • Keywords review? added

Some more pylint with my questions.

pylint output:
[root@localhost sugar]# sh pylint.sh
* Module shell.shellservice
E0602:119:ShellService.CurrentActivityChanged: Undefined variable 'iface'
* Module sugar.network
E0602:549:start_xmlrpc: Undefined variable 'loop'
* Module sugar.graphics.palette
E0602:494:PaletteActionBar.add_action: Undefined variable 'Button'
E0602:497:PaletteActionBar.add_action: Undefined variable 'Icon'
E0602:501:PaletteActionBar.add_action: Undefined variable 'self'

_

"iface" undefined.

sugar-jhbuild/source/sugar/shell/shellservice.py line 119 :

def CurrentActivityChanged(self, activity_id):

if os.path.exists('/etc/olpc-security'):

self._get_rainbow_service().ChangeActivity(activity_id, dbus_interface=iface)

I have no clue how to fix this one.

"loop" is undefined. Also, why is the call to this function commented out?

sugar-jhbuild/source/sugar/sugar/network.py line 549

def start_xmlrpc():

server = GlibXMLRPCServer(("", 8888))
inst = Test()
server.register_instance(inst)
gobject.idle_add(xmlrpc_test, loop)

Maybe calling this function with the "loop". But, is this unused since it's commented out in main()? Should the code be removed?
_

"Button", "Icon", "self" are undefined.

sugar-jhbuild/source/sugar/sugar/graphics.py lines 494, 497, 501

class PaletteActionBar(gtk.HButtonBox):

def add_action(label, icon_name=None):

button = Button(label)

if icon_name:

icon = Icon(icon_name)
button.set_image(icon)
icon.show()

self.pack_start(button)
button.show()

Don't know what to do about Button and Icon. Also the call to this function I think should be:
class PaletteActionBar(self,gtk.HButtonBox):

comment:8 Changed 7 years ago by mstone

  • Cc mstone added

comment:9 Changed 7 years ago by marco

  • Keywords review? removed

comment:10 Changed 7 years ago by erikos

  • Cc tomeu marco erikos added
  • Owner changed from dcbw to erikos

I went through the errors I get with pylint for the sugar shell. Here are the ones we get false errors.

  • hippocanvas

python/hippo.override
change line 30 to 'hippo'
Action item: marco can you fix this upstream?

  • dbus conversion
    • dbus.String(self._key)
    • E1102: dbus.String is not callable
    • works in 0.41
  • Message has unused arguments
    • W0613: Unused argument 'arg0'

<example>
def new_picture(self, playa, pixbuf):

self.emit('pixbuf', pixbuf)

</example>

  • workaround: prefix method with 'cb_' or append '_cb', this seems hardcoded in the pylint code why I think this can not be customized to match for example ''
  • gobject
    • E1101: Instance of 'LiveVideoSlot' has no 'emit' member same behavior for 'connect', 'disconnect', 'notify', props

What does not work:
<example>
class LiveVideoSlot(gobject.GObject):

gsignals = {

'pixbuf': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, ([gobject.TYPE_PYOBJECT])),

}
def init(self, width, height):

gobject.GObject.init(self)

def _new_picture_cb(self, playa, pixbuf):

self.emit('pixbuf', pixbuf)

</example>

What does work:
<example>
class LiveVideoSlot(gtk.EventBox):

gsignals = {

'pixbuf': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, ([gobject.TYPE_PYOBJECT])),

}
def init(self, width, height):

gtk.EventBox.init(self)

def _new_picture_cb(self, playa, pixbuf):

self.emit('pixbuf', pixbuf)

</example>

  • vbox keydialog
    • E1101: 96:KeyDialog.init: Class 'vbox' has no 'pack_start' member

self.vbox.module is gtk
<example>
class KeyDialog(gtk.Dialog):

def init(self):

gtk.Dialog.init(self, flags=gtk.DIALOG_MODAL)
self.set_title("Wireless Key Required")
label = gtk.Label("Label")
self.vbox.pack_start(label)

</example>

  • gtk.Window

E1101: 10:ActivityChatWindow.init: Class 'window' has no 'set_type_hint' member
E1101: 11:ActivityChatWindow.init: Class 'window' has no 'set_accept_focus' member

calling self.set_type_hint() works for pylint
self.window.module is gtk.gdk
<example>
class ActivityChatWindow(gtk.Window):

def init(self):

gtk.Window.init(self)

self.realize()
self.set_decorated(False)
self.window.set_type_hint(gtk.gdk.WINDOW_TYPE_HINT_DIALOG)
self.window.set_accept_focus(True)

ActivityChatWindow()
</example>

  • child

E1101: 10:ClipboardIcon.init: Class 'child' has no 'connect' member

calling self.connect() works for pylint
self.child is of type GtkRadioButton and module is gtk

<example>
from sugar.graphics.radiotoolbutton import RadioToolButton

class ClipboardIcon(RadioToolButton):

gtype_name = 'SugarClipboardIcon'

def init(self):

RadioToolButton.init(self)
self.child.connect('drag_data_get', self._drag_data_get_cb)

def _drag_data_get_cb(self, widget, context, selection, targetType, eventTime):

pass

</example>

  • sugar.src.view.BuddyMenu (GtkMenuShell)

E1101: 65:BuddyMenu._add_items: Instance of '_Menu' has no 'append' member

palette.menu.module

'sugar.graphics.palette'

comment:11 Changed 7 years ago by erikos

(this comment is better formatted)

I went through the errors I get with pylint for the sugar shell. Here are the ones we get false errors.

  • hippocanvas
    • python/hippo.override
    • change line 30 to 'hippo'
    • Action item: marco can you fix this upstream?
  • dbus conversion
    • dbus.String(self._key)
    • E1102: dbus.String is not callable
    • works in 0.41
  • Message has unused arguments
    • W0613: Unused argument 'arg0'
      def __new_picture(self, playa, pixbuf):
              self.emit('pixbuf', pixbuf)
      
    • workaround: prefix method with 'cb_' or append '_cb', this seems hardcoded in the pylint code why I think this can not be customized.
  • gobject
    • E1101: Instance of 'LiveVideoSlot' has no 'emit' member
    • same behavior for 'connect', 'disconnect', 'notify', props

What does not work:

class LiveVideoSlot(gobject.GObject):
    __gsignals__ = {
        'pixbuf': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, ([gobject.TYPE_PYOBJECT])),
    }
    def __init__(self, width, height):
        gobject.GObject.__init__(self)

    def _new_picture_cb(self, playa, pixbuf):
        self.emit('pixbuf', pixbuf)

What does work:

class LiveVideoSlot(gtk.EventBox):
    __gsignals__ = {
        'pixbuf': (gobject.SIGNAL_RUN_FIRST, gobject.TYPE_NONE, ([gobject.TYPE_PYOBJECT])),
    }
    def __init__(self, width, height):
        gtk.EventBox.__init__(self)

    def _new_picture_cb(self, playa, pixbuf):
        self.emit('pixbuf', pixbuf)
  • vbox keydialog
    • E1101: 96:KeyDialog.init: Class 'vbox' has no 'pack_start' member
    • self.vbox.module is gtk
      class KeyDialog(gtk.Dialog):
          def __init__(self):
              gtk.Dialog.__init__(self, flags=gtk.DIALOG_MODAL)
              self.set_title("Wireless Key Required")
              label = gtk.Label("Label")
              self.vbox.pack_start(label)
      
  • gtk.Window
    • E1101: 10:ActivityChatWindow.init: Class 'window' has no 'set_type_hint' member
    • E1101: 11:ActivityChatWindow.init: Class 'window' has no 'set_accept_focus' member
    • calling self.set_type_hint() works for pylint
    • self.window.module is gtk.gdk
      class ActivityChatWindow(gtk.Window):
          def __init__(self):
              gtk.Window.__init__(self)
      
              self.realize()
              self.set_decorated(False)
              self.window.set_type_hint(gtk.gdk.WINDOW_TYPE_HINT_DIALOG)
              self.window.set_accept_focus(True)
      ActivityChatWindow()
      
  • child
    • E1101: 10:ClipboardIcon.init: Class 'child' has no 'connect' member
    • calling self.connect() works for pylint
    • self.child is of type GtkRadioButton and module is gtk
from sugar.graphics.radiotoolbutton import RadioToolButton

class ClipboardIcon(RadioToolButton):
    __gtype_name__ = 'SugarClipboardIcon'

    def __init__(self):
        RadioToolButton.__init__(self)
        self.child.connect('drag_data_get', self._drag_data_get_cb)

    def _drag_data_get_cb(self, widget, context, selection, targetType, eventTime):
        pass
  • sugar.src.view.BuddyMenu (GtkMenuShell)
    • E1101: 65:BuddyMenu._add_items: Instance of '_Menu' has no 'append' member
      palette.menu.__module__
      'sugar.graphics.palette'
      

comment:12 Changed 7 years ago by erikos

regarding the gobject issue i got following reply from the pylint devs:

i think this is due to a known pb with the way astng is built from
living object (which is necessary when object is coming from C code):
if object's __module__ attribute isn't equivalent to the actual (python)
name of the compiled module, it's missing an important part of available
information. E.g. since :

>>> >>> GObject.__module__
'gobject'

while GObject is actually defined in gobject._gobject, and the same
problem occurs much probably on other objects.
The simpliest way to fix this is to report a bug on the pygtk tracker.

comment:13 Changed 7 years ago by erikos

The pygobject issue in their bugzilla tracker.

http://bugzilla.gnome.org/show_bug.cgi?id=523821

comment:14 Changed 7 years ago by erikos

Johan Dahlin pushed a fix to pygobject svn that fixes the pygobject false errors.

comment:15 Changed 7 years ago by marco

I fixed hippo canvas.

Note: See TracTickets for help on using tickets.