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

Closed
evidencebp opened this issue Sep 24, 2024 · 6 comments
Closed

Pylint alerts corrections as part of intervention experiment #81

evidencebp opened this issue Sep 24, 2024 · 6 comments

Comments

@evidencebp
Copy link
Contributor

I'd like to conduct a software engineering experiment regarding the benefit of Pylint alerts removal.
The experiment is described here.
In the experiments, Pylint is used with some specific alerts, files are selected for intervention and control.
After the interventions are done, one can wait and examine the results.

I'm asking for your approval for conducting the interventions in your repositories.The interventions are expected to benefit the project and will be submitted in PR for approval.

These are the planed interventions.

@gabfl @huangyunict @bubak4, may I fix the alerts?

@gabfl
Copy link
Owner

gabfl commented Sep 24, 2024

@evidencebp yes please feel free

@evidencebp
Copy link
Contributor Author

Great, Thanks!

@evidencebp
Copy link
Contributor Author

I created the PR.

However, I had a problem with the relative imports so I could not run neither the code nor the tests.
When possible, a checked the changes externally and all of them are rather minor.

Can you please:

  • Guide me with the relative imports
  • Take a look at the PR and tell me what do you think.

@gabfl

@evidencebp
Copy link
Contributor Author

When running c:/src/vault/src/main.py
I get the error below.
I googled it and played around but could not solve it.

PS C:\src\vault> & C:/Users/USER/anaconda3/envs/vault/python.exe c:/src/vault/src/main.py
Traceback (most recent call last):
File "c:\src\vault\src_main_.py", line 1, in
from . import vault
ImportError: attempted relative import with no known parent package

gabfl pushed a commit that referenced this issue Sep 25, 2024
* src\views\setup.py  line-too-long

Line was one letter too long

* src\tools\troubleshoot_db.py  simplifiable-if-expression

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

* src\views\menu.py  broad-exception-caught

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.

* src\lib\Config.py  line-too-long

Made the long, yet readable, comment shorter

* src\modules\autocomplete.py  broad-exception-caught

 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.

* src\modules\misc.py  broad-exception-caught

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.

* src\views\categories.py  superfluous-parens

Replaced (True) to True

* src\views\import_export.py  line-too-long

Made a readable comment line shorter

* src\views\migration.py  line-too-long

Made the line shorter.
Since the string is also formatted, parenthesis are added for operations precedence.
@gabfl
Copy link
Owner

gabfl commented Sep 25, 2024

The PR looks good. It's hard to run this program from Windows, I wouldn't be able to guide you, I use a unix-bases system

evidencebp added a commit to evidencebp/pylint-intervention that referenced this issue Sep 26, 2024
@evidencebp
Copy link
Contributor Author

Thank you very much!
In case that you would like to intervene yourself in other project or have ideas for such, please tell me.
Of course, if you know others that might be interested, let them know too.

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

No branches or pull requests

2 participants