Skip to content

Consider adding additional linters to build #22

Closed

Description

@hacst

pylama supports additional linters we might want to make use of besides pep8. I will add pyflakes support as it detects common errors like unused imports or naming errors. Others aren't so clear cut:

There's mccabe for code complexity checking. Enabling currently yields the following:

src\modes.py:25:1: C901 'generate_mode_base' is too complex (16) [mccabe]
src\simulation_model.py:415:1: C901 'JsonMeta.validate_data_from_spec' is too complex (15) [mccabe]
src\algorithms\hightower.py:98:1: C901 'hightower_line_search' is too complex (69) [mccabe]
src\logicitems\linetree.py:47:1: C901 'LineTree._update_tree' is too complex (13) [mccabe]
src\schematics\mouse_modes\line_submode\inserting.py:80:1: C901 'InsertingLineSubMode.do_update_line' is too complex (40) [mccabe]

I think its got good point with all of these. Imho it would make sense to enable it.

There's also pep257 which defines how docstrings ought to look like. Here's a snippet of what it complains about:

...
src\item_list_widget.py:1:1: D200: One-line docstring should not occupy 3 lines [pep257]
src\item_list_widget.py:1:1: D300: Expected """-quotes, got '''-quotes [pep257]
src\item_list_widget.py:17:1: D101: Docstring missing [pep257]
...
src\settings.py:121:1: D205: Blank line missing between one-line summary and description [pep257]
src\settings.py:121:1: D401: First line should be imperative: 'Return', not 'Returns' [pep257]
src\settings.py:132:1: D401: First line should be imperative: 'Initialize', not 'Initializes' [pep257]
src\setup.py:1:1: D200: One-line docstring should not occupy 3 lines [pep257]
src\setup.py:1:1: D300: Expected """-quotes, got '''-quotes [pep257]
...

Some of those might be useful but I'm not sure being that strict about doc strings is worth the hassle.

Do we want to have all these running as part of the build? Do we like to enable a subset of what they report?

You can play around with them by adding the linters to the pylama.ini:

[pylama]
linters = pep8,pyflakes,pep257,mccabe

Activity

  1. hacst commented on Nov 21, 2014

    @hacst
    MemberAuthor

    If we enable mccabe we'll likely want to raise the allowed complexity a bit. The default seems overly strict

    [pylama:mccabe]
    complexity = 12

    Seems a reasonable compromise (maybe one or two more). Currently that returns:

    src\modes.py:25:1: C901 'generate_mode_base' is too complex (16) [mccabe]
    src\simulation_model.py:415:1: C901 'JsonMeta.validate_data_from_spec' is too complex (15) [mccabe]
    src\algorithms\hightower.py:98:1: C901 'hightower_line_search' is too complex (69) [mccabe]
    src\logicitems\linetree.py:47:1: C901 'LineTree._update_tree' is too complex (13) [mccabe]
    src\schematics\mouse_modes\line_submode\inserting.py:79:1: C901 'InsertingLineSubMode.do_update_line' is too complex (21) [mccabe]
    

    Those look quite reasonable to me.

  2. christianbrugger commented on Nov 25, 2014

    @christianbrugger
    Member

    I fully agree with you. mccabe seems to indicate some useful cases for refactoring, while the docstring pep257 is overly stringent in my opinion.

  3. hacst commented on Nov 25, 2014

    @hacst
    MemberAuthor

    Awesome. I created #30 as a follow-up for enabling mcabbe. Anything from pep257 you want to have checked? Otherwise we can close this issue.

  4. christianbrugger commented on Nov 25, 2014

    @christianbrugger
    Member

    In my opinion we have all the important linters.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions