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: Remove pylint pragma #260

Merged
merged 28 commits into from
Mar 1, 2024
Merged

Conversation

gfieni
Copy link
Contributor

@gfieni gfieni commented Mar 1, 2024

This PR removes the Pylint pragma (# pylint: disable=) usages across the codebase.

This check prevent us to gracefully handle errors inside the actors.
The `simple` modules have multiple problems:
- They never should have been in the powerapi core.
- Too much code for too little benefit compared to the actors tests.
- Most of the tests needed specific behaviour that was too different from
  the base actor. Thus needing even more tests for the `simple` modules...
- Because of the significant difference in the behaviour between real
  actors and the `simple` ones, the tests were not able to assess the
  performance of the actors pipeline, which was initially the main goal
  of the `simple` modules.
This allows to dynamically use fixtures without adding it to the
parameter list and triggering a pylint warning about different
arguments in the override of the method.
gfieni added 2 commits March 1, 2024 17:06
In Python 3.8, using type hint on generic types (`dict[str, str]`) instead of using the
typing module (`Dict[str, str]`) will trigger the Pylint `unsubscriptable-object` error.
Copy link

sonarqubecloud bot commented Mar 1, 2024

Quality Gate Passed Quality Gate passed

Issues
30 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

Copy link

codecov bot commented Mar 1, 2024

Codecov Report

Attention: Patch coverage is 78.37838% with 8 lines in your changes are missing coverage. Please review.

Project coverage is 67.79%. Comparing base (ba8bcdf) to head (fd734cf).
Report is 10 commits behind head on master.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #260      +/-   ##
==========================================
+ Coverage   66.93%   67.79%   +0.86%     
==========================================
  Files          86       74      -12     
  Lines        3653     3413     -240     
==========================================
- Hits         2445     2314     -131     
+ Misses       1208     1099     -109     
Files Coverage Δ
src/powerapi/actor/socket_interface.py 95.78% <100.00%> (+0.09%) ⬆️
src/powerapi/cli/binding_manager.py 67.94% <ø> (ø)
src/powerapi/cli/generator.py 85.22% <ø> (+2.75%) ⬆️
src/powerapi/formula/__init__.py 100.00% <ø> (ø)
src/powerapi/message.py 77.27% <ø> (-3.06%) ⬇️
src/powerapi/actor/actor.py 51.54% <0.00%> (ø)
src/powerapi/processor/pre/k8s/k8s_monitor.py 71.11% <78.12%> (-5.98%) ⬇️

... and 4 files with indirect coverage changes

@gfieni gfieni merged commit 816d240 into master Mar 1, 2024
9 checks passed
@gfieni gfieni deleted the refactor/remove-pylint-pragma branch March 1, 2024 16:34
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