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

Refactor a number of pylint observations #3966

Closed
wants to merge 5 commits into from

Conversation

andreas-el
Copy link
Contributor

@andreas-el andreas-el commented Sep 30, 2022

Resolves #3965

Approach
Resolved some pylint errors, warnings and refactors

Pre review checklist

  • Added appropriate release note label
  • PR title captures the intent of the changes, and is fitting for release notes.
  • Commit history is consistent and clean, in line with the contribution guidelines.

Adding labels helps the maintainers when writing release notes. This is the list of release note labels.

Report
 ======
-36169 statements analysed.
+36180 statements analysed.
 
 Statistics by type
 ------------------
@@ -19,7 +19,7 @@
 
 
 
-77499 lines have been analyzed
+77518 lines have been analyzed
 
 Raw metrics
 -----------
@@ -27,13 +27,13 @@
 +----------+-------+------+---------+-----------+
 |type      |number |%     |previous |difference |
 +==========+=======+======+=========+===========+
-|code      |53322  |68.80 |53322    |=          |
+|code      |53333  |68.80 |53322    |+11.00     |
 +----------+-------+------+---------+-----------+
 |docstring |7425   |9.58  |7425     |=          |
 +----------+-------+------+---------+-----------+
-|comment   |3485   |4.50  |3485     |=          |
+|comment   |3484   |4.49  |3485     |-1.00      |
 +----------+-------+------+---------+-----------+
-|empty     |13267  |17.12 |13267    |=          |
+|empty     |13276  |17.13 |13267    |+9.00      |
 +----------+-------+------+---------+-----------+
 
 
@@ -59,11 +59,11 @@
 +===========+=======+=========+===========+
 |convention |134    |134      |134        |
 +-----------+-------+---------+-----------+
-|refactor   |154    |154      |154        |
+|refactor   |144    |154      |154        |
 +-----------+-------+---------+-----------+
-|warning    |778    |778      |778        |
+|warning    |761    |778      |778        |
 +-----------+-------+---------+-----------+
-|error      |38     |38       |38         |
+|error      |20     |38       |38         |
 +-----------+-------+---------+-----------+
 
 
@@ -78,7 +78,7 @@
 +-------------------------------+------------+
 |redefined-outer-name           |256         |
 +-------------------------------+------------+
-|too-many-arguments             |61          |
+|too-many-arguments             |60          |
 +-------------------------------+------------+
 |wrong-import-position          |46          |
 +-------------------------------+------------+
@@ -92,16 +92,12 @@
 +-------------------------------+------------+
 |import-outside-toplevel        |25          |
 +-------------------------------+------------+
-|no-value-for-parameter         |19          |
-+-------------------------------+------------+
 |import-error                   |18          |
 +-------------------------------+------------+
 |too-many-statements            |17          |
 +-------------------------------+------------+
 |attribute-defined-outside-init |17          |
 +-------------------------------+------------+
-|redefined-builtin              |16          |
-+-------------------------------+------------+
 |unused-private-member          |15          |
 +-------------------------------+------------+
 |c-extension-no-member          |14          |
@@ -114,8 +110,6 @@
 +-------------------------------+------------+
 |keyword-arg-before-vararg      |11          |
 +-------------------------------+------------+
-|inconsistent-return-statements |9           |
-+-------------------------------+------------+
 |abstract-method                |8           |
 +-------------------------------+------------+
 |unnecessary-dict-index-lookup  |7           |
@@ -126,14 +120,12 @@
 +-------------------------------+------------+
 |assert-on-string-literal       |3           |
 +-------------------------------+------------+
-|unexpected-keyword-arg         |1           |
-+-------------------------------+------------+
-|try-except-raise               |1           |
+|no-value-for-parameter         |2           |
 +-------------------------------+------------+
 
 
 
 
 ------------------------------------------------------------------
-Your code has been rated at 9.65/10 (previous run: 9.65/10, +0.00)
+Your code has been rated at 9.69/10 (previous run: 9.65/10, +0.03)

@andreas-el andreas-el assigned andreas-el and unassigned andreas-el Sep 30, 2022
@ertomatic
Copy link
Collaborator

Can one of the admins verify this patch?

@sondreso
Copy link
Collaborator

sondreso commented Oct 3, 2022

Jenkins add to whitelist

Refactored reported inconsistent return statements where (usually)
 the issue was not returning 'None' although this is implicitly done.
@andreas-el andreas-el force-pushed the refactor_pylint_observations branch from fb61e5a to b2e26be Compare October 3, 2022 10:17
Adds disable unexpected-keyword-arg to avoid false report for unittest
Refactored instance of try-except-raise to include no-op pass statement.
Refactors variables that used builtin names.
Replace Sphinx copyright variable with alias project_copyright.
Renames func argument keyword to _func.
Adds default value for _func argument.
Adds guard if _func is not provided, function should return None.
@andreas-el andreas-el force-pushed the refactor_pylint_observations branch from b2e26be to 58698cc Compare October 3, 2022 10:46
@codecov-commenter
Copy link

codecov-commenter commented Oct 3, 2022

Codecov Report

Merging #3966 (58698cc) into main (1955596) will increase coverage by 0.14%.
The diff coverage is 44.73%.

@@            Coverage Diff             @@
##             main    #3966      +/-   ##
==========================================
+ Coverage   58.04%   58.19%   +0.14%     
==========================================
  Files         544      543       -1     
  Lines       41034    40811     -223     
  Branches     3772     3716      -56     
==========================================
- Hits        23819    23750      -69     
+ Misses      16232    16084     -148     
+ Partials      983      977       -6     
Impacted Files Coverage Δ
...rc/ert/_c_wrappers/enkf/config/enkf_config_node.py 76.71% <0.00%> (-0.36%) ⬇️
src/ert/_c_wrappers/enkf/site_config.py 80.41% <ø> (ø)
src/ert/_c_wrappers/job_queue/ext_joblist.py 82.00% <0.00%> (-1.68%) ⬇️
...ert/gui/ertwidgets/analysismodulevariablespanel.py 21.69% <0.00%> (-0.21%) ⬇️
src/ert/gui/ertwidgets/models/all_cases_model.py 32.43% <0.00%> (-0.91%) ⬇️
.../tools/plot/customize/limits_customization_view.py 24.32% <0.00%> (ø)
...rc/ert/gui/tools/plot/data_type_keys_list_model.py 37.83% <0.00%> (-1.06%) ⬇️
src/ert/gui/tools/plot/plot_case_model.py 30.55% <0.00%> (-0.88%) ⬇️
src/ert/shared/ensemble_evaluator/client.py 95.00% <ø> (ø)
src/ert/gui/ertwidgets/checklist.py 12.93% <25.00%> (ø)
... and 24 more

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@andreas-el andreas-el closed this Oct 4, 2022
@andreas-el andreas-el deleted the refactor_pylint_observations branch October 4, 2022 07:07
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.

Resolve some of the Pylint reported issues
4 participants