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

Pylint alerts corrections as part of intervention experiment #81 #82

Merged
merged 9 commits into from
Sep 25, 2024

Conversation

evidencebp
Copy link
Contributor

I fixed the planed interventions
In few cases the alert was not justified, so I left it as "won't fix".

Each intervention is in a commit of its own, having the file and alert in the title.
The message explains and justify the change.

Line was one letter too long
if result_proxy.fetchall() == [(123,)] can result in only True or False, the external condition is not needed.
However, if  result_proxy.fetchall() might return Noe, the external condition can translate it to False.

The external condition hurt readability since it takes a bit to verify what it does.
bool is a simpler implementation

See
https://stackoverflow.com/questions/76094401/the-if-expression-can-be-replaced-with-test-simplifiable-if-expression
Code deliberately catches Exception, after catching KeyboardInterrupt.
However, looking at the protected code it seems that no other exception can be raised.
Hence, instead of narrowing the exception, I removed it.
Made the long, yet readable, comment shorter
 Code deliberately catches Exception, after catching KeyboardInterrupt.
However, looking at the protected code it seems that no other exception can be raised.
Hence, instead of narrowing the exception, I removed it.
Exception is too wide.
os.path.exists does not throw exceptions.
os.makedirs might throw OSError (e.g., in a bad path).

See
https://stackoverflow.com/questions/2383816/how-can-i-make-an-error-verifiy-with-os-makedirs-in-python
As extra safety, though the code checks just before for the directory, catch it too in case a different process will be able to create it before.
Made a readable comment line shorter
Made the line shorter.
Since the string is also formatted, parenthesis are added for operations precedence.
@gabfl gabfl merged commit 5476e3a into gabfl:main Sep 25, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants