Ticket #560 (new defect)

Opened 8 years ago

Last modified 6 years ago

Pylint sugar

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

Description (last modified by marco) (diff)

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

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

Change History

  Changed 8 years ago by marco

  • description modified (diff)

  Changed 8 years ago by marco

  • priority changed from normal to high

  Changed 8 years ago by marco

  • milestone changed from BTest-2 to Opportunity

  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

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.

  Changed 7 years ago by marco

  • keywords review? removed

Pushed, thanks!

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):

  Changed 7 years ago by mstone

  • cc mstone added

  Changed 7 years ago by marco

  • keywords review? removed

  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'

  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'
    

  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.

  Changed 7 years ago by erikos

The pygobject issue in their bugzilla tracker.

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

  Changed 6 years ago by erikos

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

  Changed 6 years ago by marco

I fixed hippo canvas.

Note: See TracTickets for help on using tickets.