mild-blue / txmatching

Solver for kidney pair donation matching problems.
https://txm.demo.mild.blue
Other
2 stars 2 forks source link

Make pylint behave the same on GitHub and locally #814

Closed kristinagalik closed 1 year ago

kristinagalik commented 2 years ago

V txmatching/web/__init__.py, vo funkcii create_app sa pylint stazuje ze je 51 statementov, pricom max-statements je setnuty na 50. Pylint sa vsak stazuje iba lokalne, ale nie na GitHube, aj ked na oboch miestach je setnuty rovnako.

kristinagalik commented 2 years ago
Run conda activate txmatching
Unable to create directory /github/home/.cache/pylint
Unable to create file /github/home/.cache/pylint/txmatching1.stats: [Errno 2] No such file or directory: '/github/home/.cache/pylint/txmatching1.stats'
************* Module txmatching.data_transfer_objects.hla.parsing_issue_dto
txmatching/data_transfer_objects/hla/parsing_issue_dto.py:23:0: R0902: Too many instance attributes (8/7) (too-many-instance-attributes)
txmatching/data_transfer_objects/hla/parsing_issue_dto.py:46:0: R0902: Too many instance attributes (9/7) (too-many-instance-attributes)
************* Module txmatching.utils.hla_system.hla_crossmatch
txmatching/utils/hla_system/hla_crossmatch.py:58:4: R1702: Too many nested blocks (6/5) (too-many-nested-blocks)
************* Module txmatching.patients.patient
txmatching/patients/patient.py:66:0: R0902: Too many instance attributes (9/7) (too-many-instance-attributes)
************* Module txmatching.web
txmatching/web/__init__.py:[12](https://github.com/mild-blue/txmatching/runs/7130739165?check_suite_focus=true#step:7:13)4:0: R0915: Too many statements (51/50) (too-many-statements)
************* Module txmatching.solvers.matching.matching
txmatching/solvers/matching/matching.py:23:4: R09[14](https://github.com/mild-blue/txmatching/runs/7130739165?check_suite_focus=true#step:7:15): Too many local variables (17/[15](https://github.com/mild-blue/txmatching/runs/7130739165?check_suite_focus=true#step:7:16)) (too-many-locals)
************* Module txmatching.solvers.ilp_solver.txm_configuration_for_ilp
txmatching/solvers/ilp_solver/txm_configuration_for_ilp.py:[16](https://github.com/mild-blue/txmatching/runs/7130739165?check_suite_focus=true#step:7:17):0: R0902: Too many instance attributes (11/7) (too-many-instance-attributes)
************* Module txmatching.solvers.all_solutions_solver.score_matrix_utils
txmatching/solvers/all_solutions_solver/score_matrix_utils.py:1:0: R0801: Similar lines in 2 files
==txmatching.data_transfer_objects.patients.update_dtos.recipient_update_dto:[[21](https://github.com/mild-blue/txmatching/runs/7130739165?check_suite_focus=true#step:7:22):33]
==txmatching.data_transfer_objects.patients.upload_dtos.recipient_upload_dto:[29:41]
    def __post_init__(self):
        if self.height:
            is_height_valid("recipient", self.height)
        if self.weight:
            is_weight_valid("recipient", self.weight)
        if self.year_of_birth:
            is_year_of_birth_valid("recipient", self.year_of_birth)
        if self.previous_transplants:
            is_number_of_previous_transplants_valid(self.previous_transplants) (duplicate-code)
txmatching/solvers/all_solutions_solver/score_matrix_utils.py:1:0: R0801: Similar lines in 2 files
==txmatching.data_transfer_objects.patients.update_dtos.donor_update_dto:[12:21]
==txmatching.data_transfer_objects.patients.upload_dtos.donor_upload_dto:[[24](https://github.com/mild-blue/txmatching/runs/7130739165?check_suite_focus=true#step:7:25):33]
    def __post_init__(self):
        if self.height:
            is_height_valid("donor", self.height)
        if self.weight:
            is_weight_valid("donor", self.weight)
        if self.year_of_birth:
            is_year_of_birth_valid("donor", self.year_of_birth) (duplicate-code)
-----------------------------------
Your code has been rated at 9.98/10
Unable to create directory /github/home/.cache/pylint
Unable to create file /github/home/.cache/pylint/local_testing_utilities1.stats: [Errno 2] No such file or directory: '/github/home/.cache/pylint/local_testing_utilities1.stats'
------------------------------------
Your code has been rated at 10.00/10

^ this is an output from pylint on GitHub, it just continues to next job (as it is supposed to I assume), so @kubantjan I'm not sure whether I should do such changes in the code so its rated 10/10, or its ok how it is? (idk why it didn't show this then, maybe i have just missed it or smth)

kristinagalik commented 2 years ago

@kubantjan i tried using --rcfile=.pylintrc flag like so:

          pylint txmatching --rcfile=.pylintrc
          pylint local_testing_utilities --rcfile=.pylintrc

and github seems to find this file (because when i tried to allocate the file somewhere else, like ../.pylintrc it reported that there is no such file) but chooses to ignore or maybe override the configuration? the only instance that i found on which it fails (when i didnt make any changes yet besides the flag) was when both directories txmatching and local_testing_utilities had score under 10.0 and it fails with exit code 8. so my first thought was maybe it takes into consideration only errors from the last command from the job? so i divided linter job into two jobs, one for each directory, just to make sure i understand the issue, and even when i solved linter issues for lets say txmatching, when i used the --rcfile flag, it again failed with exit code 8 even though the code was rated 10/10. (when its rated <10 the exit code changes to 26) so it probably finishes the job and then writes the error even though the error didnt occur during execution of the last command.

jogobeny commented 2 years ago

Hi @kristinagalik,

the outputs are same. However, they are printed in different order.

I checked that on the new branch (checkouted from the current master): https://github.com/mild-blue/txmatching/actions/runs/3055809545/jobs/4929284599

(txmatching) [jogobeny@archlinux txmatching]$ whereis pylint
pylint: /home/jogobeny/.conda/envs/txmatching/bin/pylint

pylint txmatching locally:

************* Module txmatching.data_transfer_objects.optimizer.optimizer_in_swagger
txmatching/data_transfer_objects/optimizer/optimizer_in_swagger.py:7:0: W0613: Unused argument 'args' (unused-argument)
************* Module txmatching.database.services.report_service
txmatching/database/services/report_service.py:13:0: C0411: standard import "from distutils.dir_util import copy_tree" should be placed before "import jinja2" (wrong-import-order)
************* Module txmatching.optimizer.optimizer_return_object
txmatching/optimizer/optimizer_return_object.py:2:0: W0611: Unused Dict imported from typing (unused-import)
txmatching/optimizer/optimizer_return_object.py:2:0: W0611: Unused Optional imported from typing (unused-import)
************* Module txmatching.scorers.hla_additive_scorer
txmatching/scorers/hla_additive_scorer.py:46:4: R0911: Too many return statements (9/6) (too-many-return-statements)
************* Module txmatching.web.api.matching_api
txmatching/web/api/matching_api.py:5:0: W0611: Unused request imported from flask (unused-import)
************* Module txmatching.web.api.optimizer_api
txmatching/web/api/optimizer_api.py:31:8: W0612: Unused variable 'optimizer_request_object' (unused-variable)
txmatching/web/api/optimizer_api.py:30:4: R0201: Method could be a function (no-self-use)
txmatching/web/api/optimizer_api.py:41:0: E0102: class already defined line 26 (function-redefined)
txmatching/web/api/optimizer_api.py:56:4: R0201: Method could be a function (no-self-use)
txmatching/web/api/optimizer_api.py:5:0: W0611: Unused require_valid_txm_event_id imported from txmatching.auth.auth_check (unused-import)
txmatching/web/api/optimizer_api.py:2:0: C0411: standard import "from typing import Optional" should be placed before "from flask_restx import Resource" (wrong-import-order)

-----------------------------------
Your code has been rated at 9.97/10

pylint on GitHub:

************* Module txmatching.scorers.hla_additive_scorer
txmatching/scorers/hla_additive_scorer.py:46:4: R0911: Too many return statements (9/6) (too-many-return-statements)
************* Module txmatching.web.api.optimizer_api
txmatching/web/api/optimizer_api.py:31:8: W0612: Unused variable 'optimizer_request_object' (unused-variable)
txmatching/web/api/optimizer_api.py:30:4: R0201: Method could be a function (no-self-use)
txmatching/web/api/optimizer_api.py:41:0: E0102: class already defined line 26 (function-redefined)
txmatching/web/api/optimizer_api.py:56:4: R0201: Method could be a function (no-self-use)
txmatching/web/api/optimizer_api.py:5:0: W0611: Unused require_valid_txm_event_id imported from txmatching.auth.auth_check (unused-import)
txmatching/web/api/optimizer_api.py:2:0: C0411: standard import "from typing import Optional" should be placed before "from flask_restx import Resource" (wrong-import-order)
************* Module txmatching.web.api.matching_api
txmatching/web/api/matching_api.py:5:0: W0611: Unused request imported from flask (unused-import)
************* Module txmatching.data_transfer_objects.optimizer.optimizer_in_swagger
txmatching/data_transfer_objects/optimizer/optimizer_in_swagger.py:7:0: W0613: Unused argument 'args' (unused-argument)
************* Module txmatching.optimizer.optimizer_return_object
txmatching/optimizer/optimizer_return_object.py:2:0: W0611: Unused Dict imported from typing (unused-import)
txmatching/optimizer/optimizer_return_object.py:2:0: W0611: Unused Optional imported from typing (unused-import)
************* Module txmatching.database.services.report_service
txmatching/database/services/report_service.py:13:0: C0411: standard import "from distutils.dir_util import copy_tree" should be placed before "import jinja2" (wrong-import-order)

-----------------------------------
Your code has been rated at 9.97/10

pylint local_testing_utilities locally:

------------------------------------
Your code has been rated at 10.00/10

pylint local_testing_utilities on GitHub:

------------------------------------
Your code has been rated at 10.00/10
jogobeny commented 1 year ago

So, the improvement should be that if pylint fails (score < 10), there will be no execution of tests.

jogobeny commented 1 year ago

@kristinagalik, is it okay to create own workflow in .git/workflows/ and do tests on it?

kristinagalik commented 1 year ago

@jogobeny yes, i think so

kubantjan commented 1 year ago

@krllstdn found out that the inconsistency is still there. Please describe the issue here

krllstdn commented 1 year ago

Yep, sorry, I forgot to write it here. When I worked on some task, there was an inconsistency between what I had locally and what was in github pipelines. Locally after running make check there were no errors, but in GH actions there was an error. When I tried to replicate it by literally copying the same line that caused the error in pipelines before (#1085), but wasn't noticed by local pylint, It now was considered as an error. So false alarm :)

kubantjan commented 1 year ago

@krllstdn so we can close it again? If yes, please close it :)