kamilion/customizer

Programming errors check by pylint

Closed this issue · 3 comments

This is part of issue #129 that aims to check for programming errors in Python source code using Pylint. While reviewing Makefile, I got curious with the function lint that calls for pylint and so I tried running make lint to see the output. Guess what? Need another round of clean up!

How to use pylint

For individual files, open Terminal and run the command against a Python file.

pylint FILE.py

For this project, it is possible to run pylint via makefile.

cd Customizer-master
make lint

The result (updated)

To summarize how much errors I am seeing, I have copy-paste the output by pylint (earlier, I had used grep and wc -l to filter and count the output, which is supposed to tally with report by pylint).

Messages by category
--------------------

+-----------+-------+---------+-----------+
|type       |number |previous |difference |
+===========+=======+=========+===========+
|convention |220    |NC       |NC         |
+-----------+-------+---------+-----------+
|refactor   |15     |NC       |NC         |
+-----------+-------+---------+-----------+
|warning    |40     |NC       |NC         |
+-----------+-------+---------+-----------+
|error      |13     |NC       |NC         |
+-----------+-------+---------+-----------+

In above output, pylint had excluded 2 instances of [F]atal message due to import-error and includes 13 [E]rror messages due to misleading error of syntax-error caused by regex notation (hinted in one of comments below).

Global evaluation
-----------------
Your code has been rated at 7.31/10

At a glance, the code rating for this project looks good by figures. However, code rating can also be translated to the ratio of error-free to total lines of codes.

For example, if there are 100 lines of code (excluding empty lines), there are about 70 lines of code which are error-free or considered clean and 30 lines of code contain errors. If this were true, this is not a big issue. But this project has over 1,000 lines of code, which is 10 times more than the example. That means 300 lines or roughly one-third of total lines of code contain errors. That's a lot.

Can I ignore

At beginning, I did run Pylint against source code from the first release when this project was rewritten in Python until to this date (4.1.0, 4.1.1, 4.1.2-0, 4.1.3-0, 4.1.4-0), and even made relative comparison of those. But the number of programming errors is more or less unchanged.

Would that mean the programming errors I am seeing now can be ignored? If indeed so, I could ignore this issue and focus on existing issues that I have knowledge and planned earlier i.e. Bring back 'About' tab in GUI and such. Or not, since this issue managed to steal my attention.

Correction

First release that was rewritten in Python was 4.1.0, and it was not 4.1.1. Untagged releases of 4.1.1 between 4.1.1 and 4.1.2 have been excluded.

Actual stats for errors

When running make lint command for this project, Makefile actually excludes gui.py.in and convention error (line-too-long). For this reason, I have not been able to confirm by calculating the cumulative errors when running pylint against each Python script files.

While actual stats for most errors are more or less unchanged, [C]onvention errors is now increased by 1.6 times from 135 to 220 instances.

Running make lint had additional instances for [E]rrors for programming errors (syntax-error) for no apparent reason. There are two related factors.

  1. Several Python scripts with .py.in file extension are using @PREFIX in the hashbang line so that some variables are substituted accordingly when building using Makefile. This is not recognized by pylint and thus, can be ignored.

  2. The hashbang line itself is not recognized by pylint in some situations. I know this doesn't make sense at time of finding, but there is related post on Stack Overflow that had discussed the discrepancy.

This phenomenon cannot be explained by myself.

It is dreadfully boring to fix things that almost nobody care about, so I will be lazy and target to fully resolve this issue by final milestone 4.2.0.

Note to myself @clearkimura: To make this issue less boring, include statistics for code rating for this project and follow up relative changes for every releases of "old stable" from 4.1.0 until 4.2.0 and see if there is any improvement over time.

The actual reason for above phenomenon is not apparent.

One thing for sure, pylint will always show (syntax-error) when specifying multiple files at once. I managed to reproduce this issue twice, one time using make lint command provided by this project and another time using my own script that does similarly.

Using make lint

cd src && pylint lib/* actions/*.py main.py.in gui.py.in \
		| grep -v -e 'Line too long'
************* Module lib
C:  1, 0: Missing module docstring (missing-docstring)
************* Module lib.config
C:  1, 0: Missing module docstring (missing-docstring)
W:  6, 0: No exception type(s) specified (bare-except)
F:  5, 4: Unable to import 'configparser' (import-error)
C: 11, 0: Invalid constant name "conf" (invalid-name)
************* Module lib.config.py
E:  1, 0: invalid syntax (syntax-error) <-- this one here!
...

Using make lint with workaround

cd src && pylint lib/* actions/*.py main.py.in gui.py.in \
		| grep -v -e 'invalid syntax'
No config file found, using default configuration
************* Module lib
C:  1, 0: Missing module docstring (missing-docstring)
************* Module lib.config
C:  1, 0: Missing module docstring (missing-docstring)
W:  6, 0: No exception type(s) specified (bare-except)
F:  5, 4: Unable to import 'configparser' (import-error)
C: 11, 0: Invalid constant name "conf" (invalid-name)
************* Module lib.config.py   <-- misleading (syntax-error) is now hidden!
************* Module lib.message
C: 50, 0: Unnecessary parens after 'print' keyword (superfluous-parens)
...

Using own script

...
Run pylint for all files at once...
Change directory: /home/trusty/test/411+/4.1.4-155/src
Command: pylint lib/* actions/* main.py.in gui.py.in
Save to /home/trusty/test/411+/4.1.4-155/4.1.4-155.log
...
~/test/411+$ head ./4.1.4-155/4.1.4-155.log
...
************* Module lib.config
C:  1, 0: Missing module docstring (missing-docstring)
W:  6, 0: No exception type(s) specified (bare-except)
F:  5, 4: Unable to import 'configparser' (import-error)
C: 11, 0: Invalid constant name "conf" (invalid-name)
************* Module lib.config.py
E:  1, 0: invalid syntax (syntax-error) <-- this one again!
************* Module lib
C:  1, 0: Missing module docstring (missing-docstring)
************* Module lib.message
...

Such error will not appear when pylint is run against each file.

Command: pylint ./src/lib/config.py
Save to ./src/lib/config.py.log
...
~/test/411+$ head ./4.1.4-155/src/lib/config.py.log 
************* Module lib.config
C:  1, 0: Missing module docstring (missing-docstring)
W:  6, 0: No exception type(s) specified (bare-except)
F:  5, 4: Unable to import 'configparser' (import-error)
C: 11, 0: Invalid constant name "conf" (invalid-name)


Report
======
26 statements analysed.
        ^ huh, no more (syntax-error) for unexplained reason
...

I have designed my own script to run pylint twice (first time against each files, second time against multiple files that is similar to make lint and clear stats files in ~/.pylint.d each time before run). This way, I can easily run diff to find any misleading errors produced by pylint.

As such, I will use my own script to check from 4.1.0 until 4.2.0.

Updated 2017.01.23

Most likely, the regex notation asterisk * is the cause pylint showing those additional errors. I think I did manage to eliminate the errors in my own script earlier by using variable to contain list of path to each files, before changing to regex approach like Makefile.

A quick workaround for hiding the misleading error in Makefile is, to amend from $(GREP) -v -e 'Line too long' to target error message $(GREP) -v -e 'invalid syntax'. The former is easier to resolve, so no need to filter that. Also, gui.py.in is now included in lint check.

How to disable pylint warning

Example code with pylint output

#!/usr/bin/python2.7
...    
conf = ConfigParser.SafeConfigParser(
    {
    ...
    }
)
...

$ pylint config.py 
No config file found, using default configuration
************* Module lib.config
C:  1, 0: Missing module docstring (missing-docstring)
W:  6, 0: No exception type(s) specified (bare-except)
F:  5, 4: Unable to import 'configparser' (import-error)
C: 11, 0: Invalid constant name "conf" (invalid-name)


Report
======
...

Method 1: pylint message control

pylint --disable=<msg ids>

When using command line option to disable missing-docstring error for no docstring, there won't be any line appearing at all.

$ pylint --disable=missing-docstring config.py 
No config file found, using default configuration
************* Module lib.config
W:  6, 0: No exception type(s) specified (bare-except)
F:  5, 4: Unable to import 'configparser' (import-error)
C: 11, 0: Invalid constant name "conf" (invalid-name)


Report
======
...

This is the straightfoward approach to ignore any particular error at run time and to focus on more important errors to be resolved.

Method 2: pylint in-line comment

When using in-line comment to disable errors, the original errors i.e [C]onvention messages are converted into [I]nformation messsages with locally-disabled status.

#!/usr/bin/python2.7
# pylint: disable=missing-docstring
...

conf = ConfigParser.SafeConfigParser( # pylint: disable=invalid-name
    {
    ...
    }
)
...

$ pylint config.py 
No config file found, using default configuration
************* Module lib.config
I:  2, 0: Locally disabling C0111 (locally-disabled)
I: 12, 0: Locally disabling C0103 (locally-disabled)
W:  7, 0: No exception type(s) specified (bare-except)
F:  6, 4: Unable to import 'configparser' (import-error)

One advantage for this approach is, the problematic lines could still be identified when running pylint by default, and disabled instances will minimize poor rating for global evaluation (only increases score by 0.1 or so, very trivial improvement).

This is more like a workaround to indicate there is something that should be fixed, but not resolved yet.

Combining with method 1, the converted [I]nformation messages can be hidden all together by --disable=<msg ids>.

$ pylint --disable=locally-disabled config.py 
No config file found, using default configuration
************* Module lib.config
W:  7, 0: No exception type(s) specified (bare-except)
F:  6, 4: Unable to import 'configparser' (import-error)


Report
======
...

If that is not enough, the converted less important messages can also be hidden by using pylint error mode argument --errors-only.

> --errors-only, -E
>     In error mode, checkers without error messages are disabled and
>     for others, only the  ERROR  messages  are  displayed,  and  no
>     reports are done by default

As a result, there is usually nothing important or no critical errors to fix (good) but that defeats the purpose of cleaning up the code (bad).

$ pylint --errors-only config.py 
No config file found, using default configuration
$

Remarks

Earlier, I had made a commit for quick workaround to hide the misleading error. I agree with the approach used by the original developer using grep because the code has nothing wrong at first place, and only occurs when not parsing the full path to Python script files to pylint.

I couldn't think other approach to already-convenient regex notation in Makefile. Perhaps newer releases of pylint doesn't have the misleading error anymore? I am still running 14.04 release, which includes pylint 1.1.0. In comparison, newer LTS release include pylint 1.5 and above.

This issue shall be closed, because "old stable" will be deprecated. No need to fix this boring issue anymore.