Ensure code meets pylint standards

[SPlanzer]

GitHub actions should test all pull requests to ensure pylint standards are meet.

[stale[bot]]

This issue has been automatically marked as stale as there has not been any activity for sometime. The issue will be closed in 14 days if no further activity.

[SPlanzer]

FYI @billgeo As discussed at this weeks planning session - A pylint report for the gazetter application.

As mentioned this is a large piece of work. The report confirms this. Pylint is requesting 4217 changes to be made.

Below (contained in the collapsible section) is a very small subsection of the report. The entire report is very large.

Click to see report

``` $ pylint NZGBplugin/ ************* Module NZGBplugin NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:712:4: C0103: Method name "setNameId" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:712:4: C0103: Argument name "id" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:712:4: C0111: Missing method docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:712:24: W0622: Redefining built-in 'id' (redefined-builtin) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:715:4: C0103: Method name "getNameId" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:715:4: C0111: Missing method docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:718:4: C0103: Method name "getName" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:718:4: C0111: Missing method docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:721:4: C0111: Missing method docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:724:4: C0103: Method name "showName" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:724:4: C0103: Argument name "id" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:724:4: C0111: Missing method docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:724:23: W0622: Redefining built-in 'id' (redefined-builtin) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:731:4: C0103: Method name "closeEvent" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:731:4: C0111: Missing method docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:735:8: E0602: Undefined variable 'QDockWidget' (undefined-variable) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:33:0: W0611: Unused import DatabaseConfiguration (unused-import) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:35:0: C0411: first party import "from LINZ.gazetteer import Model" should be placed before "from . import DatabaseConfiguration" (wrong-import-order) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:36:0: C0411: first party import "from LINZ.gazetteer import Database" should be placed before "from . import DatabaseConfiguration" (wrong-import-order) NZGBplugin/LINZ/gazetteer/gui/NameWebView.py:37:0: C0411: first party import "from LINZ.Util import pyratemp, dms" should be placed before "from . import DatabaseConfiguration" (wrong-import-order) ************* Module NZGBplugin.LINZ.gazetteer.gui.Config NZGBplugin/LINZ/gazetteer/gui/Config.py:16:0: C0327: Mixed line endings LF and CRLF (mixed-line-endings) NZGBplugin/LINZ/gazetteer/gui/Config.py:18:16: C0326: Exactly one space required around assignment organisationName='Land Information New Zealand' ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:19:15: C0326: Exactly one space required around assignment applicationName='Gazetteer Administration' ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:20:9: C0326: Exactly one space required around assignment _settings=None ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:25:29: C0326: No space allowed after bracket _settings = QSettings( organisationName, applicationName ) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:25:65: C0326: No space allowed before bracket _settings = QSettings( organisationName, applicationName ) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:28:7: C0326: No space allowed after bracket def set( item, value ): ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:28:21: C0326: No space allowed before bracket def set( item, value ): ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:29:28: C0326: Exactly one space required after comma settings().setValue(item,value) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:31:7: C0326: No space allowed after bracket def get( item, default='' ): ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:31:26: C0326: No space allowed before bracket def get( item, default='' ): ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:32:9: C0326: Exactly one space required around assignment value=settings().value(item,default) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:32:31: C0326: Exactly one space required after comma value=settings().value(item,default) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:34:13: C0326: Exactly one space required around assignment value=value.toString() ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:38:10: C0326: No space allowed after bracket def remove( item ): ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:38:17: C0326: No space allowed before bracket def remove( item ): ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:39:21: C0326: No space allowed after bracket settings().remove( item ) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:39:28: C0326: No space allowed before bracket settings().remove( item ) ^ (bad-whitespace) NZGBplugin/LINZ/gazetteer/gui/Config.py:1:0: C0103: Module name "Config" doesn't conform to snake_case naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/Config.py:1:0: C0111: Missing module docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/Config.py:28:0: W0622: Redefining built-in 'set' (redefined-builtin) NZGBplugin/LINZ/gazetteer/gui/Config.py:16:0: W0401: Wildcard import PyQt5.QtCore (wildcard-import) NZGBplugin/LINZ/gazetteer/gui/Config.py:18:0: C0103: Constant name "organisationName" doesn't conform to UPPER_CASE naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/Config.py:19:0: C0103: Constant name "applicationName" doesn't conform to UPPER_CASE naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/Config.py:20:0: C0103: Constant name "_settings" doesn't conform to UPPER_CASE naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/Config.py:22:0: C0111: Missing function docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/Config.py:23:4: C0103: Constant name "_settings" doesn't conform to UPPER_CASE naming style (invalid-name) NZGBplugin/LINZ/gazetteer/gui/Config.py:23:4: W0603: Using the global statement (global-statement) NZGBplugin/LINZ/gazetteer/gui/Config.py:25:20: E0602: Undefined variable 'QSettings' (undefined-variable) NZGBplugin/LINZ/gazetteer/gui/Config.py:28:0: C0111: Missing function docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/Config.py:31:0: C0111: Missing function docstring (missing-docstring) NZGBplugin/LINZ/gazetteer/gui/Config.py:38:0: C0111: Missing function docstring (missing-docstring) ************* Module NZGBplugin.LINZ.gazetteer.gui.DatabaseConfiguration NZGBplugin/LINZ/gazetteer/gui/DatabaseConfiguration.py:1:0: C0103: Module name "DatabaseConfiguration" doesn't conform to snake_case naming style (invalid-name) Your code has been rated at -2.66/10 (previous run: 0.00/10, -2.66) ```

[SPlanzer]

entire report attached

pylint.txt

[SPlanzer]

Some of these issues such as white space violation will be automatically fixed by Black. The majority will not

[billgeo]

Thanks @SPlanzer. Is this with the default pylint rules or some rules excluded? Just wondering if we can ignore some of this to make it more manageable.

Don't know what we should/shouldn't ignore though. Any thoughts on that? @SPlanzer @strk ?

[SPlanzer]

Thanks @SPlanzer. Is this with the default pylint rules or some rules excluded?

No Pylint exclusions provided when generating the report.

Don't know what we should/shouldn't ignore though. Any thoughts on that? @SPlanzer @strk ?

A few things at a quick glance: C0103: invalid-name: ~700 of these. (example: Module name "FormUtils" doesn't conform to snake_case naming style (invalid-name) C0111: Missing method docstring: - ~340 C0326: No space allowed before bracket ~1500 (will be fixed by Black) C0303: Trailing whitespace (trailing-whitespace): ` 170 (will be fixed by Black) C0301: Line too long : ~70 (mostly fixed by Black)

I do not think it is worth changing any of these at this stage but what Black will handle

I do not think there is value of a major formatting overhaul. This will do more harm than good as it is an opportunity to introduce new bugs. The source code does not follow PyLint but existing formatting is consistent and clear.

[billgeo]

Thanks @SPlanzer. Maybe we can leave this until later then. Especially snake case. Not sure about docstrings though. Any thoughts on that @strk ?

Also, when you say

will be fixed by pylint

do you mean 'fixed by Black'?

[SPlanzer]

do you mean 'fixed by Black'?

Yeah sorry Black

I have edit original to ensure that it is clearer.

[strk]

I don't have opinions on formatting standards, other than please keep it readable... War on format is waste of time so if a single format can be ensured to be maintained by a tool I'm happy to go there, only big reformatting makes it harder to spot who last touched a line of source code (most lines would be touch by formatter) - luckly git blame as an option to ignore commits, so if such reformatting work is done in a single commit it should make things easier.

Specifically on which format to pick I really don't have an opinion, anything works for me

[billgeo]

Okay, thanks. If it already follows some kind of consistent standard, but not pylint. Let's put this on the backlog for now.

[SPlanzer]

To large to apply retrospectively