Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A whole bunch of pylint warning/error/refactor fixes #384

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

mwheeler
Copy link

@mwheeler mwheeler commented Jun 1, 2022

WARNING: This PR is pretty big, it's a LOT of small fixes all throughout the whole code base. Probably can't be reviewed in one sitting (take your time, go through one file at a time). Thankfully, it's all low risk minor changes.

This PR is specifically intended to address hundreds of pylint warnings/errors & convention guidelines to bring the PyRate code base into a more standardised set of coding paradigms & conventions.

This PR should have absolutely zero impact on processing outcomes, the code should run exactly as it did before this PR.

The primary reason for this is to enable PyRate to use pylint as an additional QA mechanism to keep the quality of the code base maintainable and less prone to error, which will help future refactors (eg: cleaning up shared.py / configuration.py / etc) by giving us an extra tool to ensure we haven't done something wrong / haven't forgotten something.

Before this PR:

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

+-----------+-------+---------+-----------+
|type       |number |previous |difference |
+===========+=======+=========+===========+
|convention |465    |465      |465        |
+-----------+-------+---------+-----------+
|refactor   |85     |85       |85         |
+-----------+-------+---------+-----------+
|warning    |156    |156      |156        |
+-----------+-------+---------+-----------+
|error      |31     |31       |31         |
+-----------+-------+---------+-----------+

After this PR:

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

+-----------+-------+---------+-----------+
|type       |number |previous |difference |
+===========+=======+=========+===========+
|convention |9      |9        |9          |
+-----------+-------+---------+-----------+
|refactor   |29     |32       |32         |
+-----------+-------+---------+-----------+
|warning    |8      |8        |8          |
+-----------+-------+---------+-----------+
|error      |4      |4        |4          |
+-----------+-------+---------+-----------+

The remaining errors/warnings I left in are legitimate problems, however out of scope (or I lack the context/PyRate-knowledge) to fix them immediately (best done in their own PRs).

…int messages from hundreds down to just 50 - the ones that remain are larger issues mostly, or things I'm not too sure on a nice way to fix just yet
…alid reason for being disabled), kept a few disabled with justification for why... and tweaked some other parameters to be more suitable for algorithmic/math heavy code in PyRate
@mwheeler mwheeler requested a review from adeane-ga June 1, 2022 00:25
@mwheeler mwheeler self-assigned this Jun 1, 2022
… cause of the unit test failure, which I still haven't found the source of)
… can find that could impact how anything runs)
… Configuration, and fixed the issue that has been identified a missing f-string in my prepifg.py refactoring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant