sberl / supersid

Cross-platform Sudden Ionospheric Disturbances (SID) monitor
MIT License
9 stars 9 forks source link

improve pylint rating #59

Open fenrog opened 2 years ago

fenrog commented 2 years ago

Currently pylint produces roughly 700 messages. This should be improved.

Please specify the pylint configuration to be used and possibly a priority of warning types a/o files to be handled first.

sberl commented 2 years ago

That 700 is down from around 1500 a few weeks ago. I'd think we should focus on errors and warning messages, and not worry too much about info? Are there any particular messages we should turn off?

Steve

On Sat, Jan 15, 2022 at 3:02 AM fenrog @.***> wrote:

Currently pylint produces roughly 700 messages. This should be improved.

Please specify the pylint configuration to be used and possibly a priority of warning types a/o files to be handled first.

— Reply to this email directly, view it on GitHub https://github.com/sberl/supersid/issues/59, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHXLENXTFF6N4SZPVYQZC3UWFH3HANCNFSM5MA3OPBQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you are subscribed to this thread.Message ID: @.***>

-- -steve

fenrog commented 2 years ago

Based on some reading of this pylint introduction, I propose the following procedure: 1) create .pylintrc pylint --generate-rcfile > .pylintrc 2) fix errors pylint -E file.py 3) fix non-comments pylint --disable=C file.py 4) fix the comments pylint file.py

fenrog commented 2 years ago

I'd disable this message: R1705: Unnecessary "else" after "return" (no-else-return). Although the else is technically not required, to me the code appears often better readable with the else and indentation.

Here it is obvious that the code takes two distinct paths.The indentation helps reading the code:

if all_ok:
    return True
else:
    print("Error: some text")
    return False

Here one has to think about the effect of the return in the if block to detect the following code will not be executed:

if all_ok:
    return True
print("Error: some text")
return False
sberl commented 2 years ago

I agree. -steve

On Sat, Jan 15, 2022 at 11:07 PM fenrog @.***> wrote:

I'd disable this message: R1705: Unnecessary "else" after "return" (no-else-return). Although the else is technically not required, to me the code appears often better readable with the else and indentation.

Here it is obvious that the code takes two distinct paths.The indentation helps reading the code:

if all_ok: return True else: print("Error: some text") return False

Here one has to think about the effect of the return in the if block to detect the following code will not be executed:

if all_ok: return True print("Error: some text") return False

— Reply to this email directly, view it on GitHub https://github.com/sberl/supersid/issues/59#issuecomment-1013823393, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAHXLEOP6SDRLT2TC4WNA2LUWJVDJANCNFSM5MA3OPBQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

You are receiving this because you commented.Message ID: @.***>

-- -steve