Switched from pycodestyle to pylint

[LordGameleo]

#173
Note: It will not pass the build. We will have to correct it. I have created new issue for it. #175

[mzfr]

I think we should have a .pylintrc too. So we can configure pylint. Because pylint have lots of options of suggestions and stuff which might not be needed.

[LordGameleo]

What I propose is:

• We need to discuss which all checks we need according to kodi's style of code.
• We can have multiple issues which focuses on reformatting the codebase according to failing pylint checks one or few at a time.
• For now I will just put all these checks in disabled state so our build passes for now and as we progress in future with these smaller issues which aims at one type of check at a time we will remove them from pylintrc file.

Benefit from doing this is:

• As these smaller issues are resolved in future, upcoming PR will be checked for them too. (Continuous Progress)
• It will create few first timers only/newcomers issues, creating opportunities for them.

@razzeee @Rechi @mzfr What do you think about it?

[LordGameleo]

I disabled these checks to get a 10 on 10 score for now.

        broad-except,
        too-few-public-methods,
        parse-error,
        invalid-name,
        attribute-defined-outside-init,
        pointless-statement,
        useless-object-inheritance,
        no-else-return,
        no-name-in-module,
        len-as-condition,
        missing-docstring,
        bad-whitespace,
        duplicate-code,
        unused-import,
        unused-variable,
        useless-else-on-loop,
        bad-continuation,
        import-error,
        too-many-arguments,
        logging-not-lazy,
        undefined-variable,
        syntax-error,
        unidiomatic-typecheck,
        too-many-nested-blocks,
        inconsistent-return-statements,
        arguments-differ,
        consider-using-in,
        deprecated-method,
        redefined-outer-name,

[LordGameleo]

#175
This issue will be broken into several parts for example:

• One issue to write missing docstrings
• One for removing unused imports and variables
• So on

[razzeee]

Sounds sensible.

My preferred setup would be to rely on the standard configuration and not configuring it at all. But getting there will take time. Am I correct that we should be able to just delete the config file then and it will just use pylint defaults?

[mzfr]

@razzeee Running pylint with default config is possible and actually isn't a big task to achieve because there aren't many errors left, the most amounts of errors are about snake_case vs the camelCase so we need to decide that and then make sure everyone use that. Other than that just the docstrings and Do not use len(SEQUENCE) to determine if a sequence is empty errors are there. Which I think won't be a problem to fix.

That being said I still think that running pylint on complete default isn't a good idea(at least in my opinion) because pylint gives quite a few suggestions of itself and sometime can have False-positives.

[LordGameleo]

@razzeee it is possible and I am also suggesting that. As we progress with individual issues we will remove them one by one from disabled list. And once all these issues are solved we will be left with the default configuration. Its like we will be doing it in progressive way.

[Rechi]

Where is the whole content from .pylintrc comming form? If most of it is the default configuration, I don't get the point in explicitly specifying it in this repo.
Also it seems like you have disabled more checks then actually needed. Here pylint succeeded with (only) 22 disabled checks.

[LordGameleo]

I disabled these checks to get a 10 on 10 score for now.

        broad-except,
        too-few-public-methods,
        parse-error,
        invalid-name,
        attribute-defined-outside-init,
        pointless-statement,
        useless-object-inheritance,
        no-else-return,
        no-name-in-module,
        len-as-condition,
        missing-docstring,
        bad-whitespace,
        duplicate-code,
        unused-import,
        unused-variable,
        useless-else-on-loop,
        bad-continuation,
        import-error,
        too-many-arguments,
        logging-not-lazy,
        undefined-variable,
        syntax-error,
        unidiomatic-typecheck,
        too-many-nested-blocks,
        inconsistent-return-statements,
        arguments-differ,
        consider-using-in,
        deprecated-method,
        redefined-outer-name,

@Rechi I just disabled these checks which was causing the build to fail.... I used the feature of pylint which creates pylintrc file. Everything except what I mentioned above is default. I used standard method of pylint to create it. We can remove all other things if we want to.

[Rechi]

This is now a more reasonable list, then you have in the current .pylintrc file.
The reason why parse-error, pointless-statement, syntax-error and undefined-variable have to be ignored are not actual code errors, but linting LICENSE, MANIFEST.in, PUBLISH.md, requirements.txt and script.test.
Also I don't have to exclude no-name-in-module, bad-whitespace and import-error.

[LordGameleo]

@Rechi for confirming.... Do you want me to make my own customized version of pylintrc file which includes only these errors which causes the build to fail.
And yes, we are ignoring these errors just so we can merge this branch successfully without failing travis build. We will solve these errors in new issues and when they are those issues are solved we will remove those disabled errors from pylintrc( specific to these disabled ones ) and we will keep ignoring those errors which we don't have to ignore.

[LordGameleo]

@razzeee @mzfr @Rechi Is there anything to change here?

[Rechi]

Exclude the mentioned files from the pylint check and remove the checks from the disabled list.

[LordGameleo]

This is now a more reasonable list, then you have in the current .pylintrc file.
The reason why parse-error, pointless-statement, syntax-error and undefined-variable have to be ignored are not actual code errors, but linting LICENSE, MANIFEST.in, PUBLISH.md, requirements.txt and script.test.
Also I don't have to exclude no-name-in-module, bad-whitespace and import-error.

@Rechi I am sorry I guess I couldn't get what you exactly said here. I got it now, I will fix these. Sorry for making you do the extra work

[LordGameleo]

@Rechi Check this once and tell me if you see anything now.

[Rechi]

pip install pylint is missing in .travis.yml.

[Rechi]

I do not have to disable this check.

[LordGameleo]

It is a problem with pylint when using virtual env. That's why it was showing this error in my pylint check. I am removing this for now from disabled list.
pylint-dev/pylint#1524

[Rechi]

Although I don't like to disable no-name-in-module and import-error, I see the reason now in travis log.
Please readd it.

[razzeee]

Can we break this up in two categories just visually.

disable=stuff-that-we-actually-want-ignored,
       stuff-that-we-actually-want-ignored2,

       stuff-thats-only-ignored-for-now-and-will-be-addressed-in-a-pr,
       stuff-thats-only-ignored-for-now-and-will-be-addressed-in-a-pr2

[LordGameleo]

@razzeee I think mostly all f them are ones which we are ignoring temporarily except import-error and no-name-in-module as It will cause build to fail due to virtual environment (as mentioned in pylint-dev/pylint#1524 )

[LordGameleo]

@razzeee @Rechi Please Review :)