Skip to content

cloudinit: remove global disable of pylint W0107 and fix errors#489

Merged
OddBloke merged 2 commits into
canonical:masterfrom
OddBloke:pylint
Jul 15, 2020
Merged

cloudinit: remove global disable of pylint W0107 and fix errors#489
OddBloke merged 2 commits into
canonical:masterfrom
OddBloke:pylint

Conversation

@OddBloke
Copy link
Copy Markdown
Collaborator

This includes removing a test class which contained no tests but wasn't
detected as empty because of an errant pass statement.

(Also update the comment in .pylintrc to match what we actually filter.)

Copy link
Copy Markdown
Collaborator

@igalic igalic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🙋🏻‍♀️


def flush(self):
"""Ensure ReportingHandler has published all events"""
pass
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i understand how the abstract methods don't need pass, but i'm surprised regular methods don't need it either

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The docstring is part of the body of the method; the pass is extraneous. Given these two definitions:

def before():
  """docstring"""
  pass


def after():
  """docstring"""

we can use Python's ast module to examine them (though note that as the parser operates on strings, before and after here are actually strings containing the above definitions):

>>> ast.dump(ast.parse(before))                                                                                                                                                                                                                                                      
"Module(body=[FunctionDef(name='flush', args=arguments(posonlyargs=[], args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[Expr(value=Constant(value='docstring', kind=None)), Pass()], decorator_list=[], returns=None, type_comment=None)], type_ignores=[])"
>>> ast.dump(ast.parse(after))
"Module(body=[FunctionDef(name='after', args=arguments(posonlyargs=[], args=[], vararg=None, kwonlyargs=[], kw_defaults=[], kwarg=None, defaults=[]), body=[Expr(value=Constant(value='docstring', kind=None))], decorator_list=[], returns=None, type_comment=None)], type_ignores=[])"

The key differences here are the body of the FunctionDef. For before, it's two statements: [Expr(value=Constant(value='docstring', kind=None)), Pass()], whereas for after it is just a single one: [Expr(value=Constant(value='docstring', kind=None))]

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Also, plugging the definitions into https://vpyast.appspot.com/ will give you a nice graphical indicator of this.)

@TheRealFalcon TheRealFalcon self-requested a review July 14, 2020 15:36
OddBloke added 2 commits July 15, 2020 09:48
This includes removing a test class which contained no tests but wasn't
detected as empty because of an errant pass statement.
@OddBloke OddBloke merged commit 4fe5765 into canonical:master Jul 15, 2020
@OddBloke OddBloke deleted the pylint branch July 15, 2020 14:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants