Improve validation logic.

.isort.cfg

@@ -1,2 +1,2 @@
 [settings]
-known_third_party = dacite,flask,flask_bcrypt,flask_restx,flask_sqlalchemy,graph_tool,gunicorn,itsdangerous,jinja2,jwt,mip,networkx,numpy,openapi_spec_validator,pandas,pdfkit,pyotp,requests,responses,sentry_sdk,sqlalchemy,swagger_unittest,werkzeug,yaml,yoyo
+known_third_party = dacite,distutils,flask,flask_bcrypt,flask_restx,flask_sqlalchemy,graph_tool,gunicorn,itsdangerous,jinja2,jwt,mip,networkx,numpy,openapi_spec_validator,pandas,pdfkit,pyotp,requests,responses,sentry_sdk,sqlalchemy,swagger_unittest,werkzeug,yaml,yoyo

local_testing_utilities/generate_patients.py

@@ -42,6 +42,8 @@
 SMALL_DATA_FOLDER_WITH_NO_SOLUTION = get_absolute_path('tests/resources/high_res_example_small_data_with_no_solution/')
 
 
+# is needed here because kw_args in dataclass is not handled very well by pylint
+# pylint: disable=unexpected-keyword-arg
 def generate_waiting_since() -> str:
     return f'{random.choice(range(2018, 2020))}-{random.choice(range(1, 12))}-{random.choice(range(1, 28))}'
 

local_testing_utilities/process_excel_data_to_json.py

@@ -30,5 +30,5 @@
             patients_together.donors += patient.donors
             patients_together.recipients += patient.recipients
 
-        with open(f'tmp.json', 'w', encoding='utf-8') as f:
+        with open('tmp.json', 'w', encoding='utf-8') as f:
             json.dump(dataclasses.asdict(patients_together), f)

txmatching/data_transfer_objects/patients/patient_base_dto.py

@@ -0,0 +1,49 @@
+from dataclasses import dataclass
+from datetime import date
+from typing import Optional
+
+from txmatching.auth.exceptions import InvalidArgumentException
+
+Kilograms = float
+Centimeters = int
+THIS_YEAR = date.today().year
+
+
+@dataclass(kw_only=True)
+class PatientBaseDTO:
+    height: Optional[Centimeters] = None
+    weight: Optional[Kilograms] = None
+    year_of_birth: Optional[int] = None
+
+    def __post_init__(self):
+        if self.weight:
+            _is_weight_valid(self.weight)
+        if self.height:
+            _is_height_valid(self.height)
+        if self.year_of_birth:
+            _is_year_of_birth_valid(self.year_of_birth)
+
+
+def _is_height_valid(height: Centimeters):
+    if height < 0:
+        raise InvalidArgumentException(f'Invalid patient height {height}cm.')
+
+
+def _is_weight_valid(weight: Kilograms):
+    if weight < 0:
+        raise InvalidArgumentException(f'Invalid patient weight {weight}kg.')
+
+
+def _is_year_of_birth_valid(year_of_birth: Centimeters):
+    if year_of_birth < 1900 or year_of_birth > THIS_YEAR:
+        raise InvalidArgumentException(f'Invalid patient year of birth {year_of_birth}')
+
+
+@dataclass(kw_only=True)
+class RecipientBaseDTO:
+    previous_transplants: Optional[int] = None
+
+    def __post_init__(self):
+        if self.previous_transplants and self.previous_transplants < 0:
+            raise InvalidArgumentException(
+                f'Invalid recipient number of previous transplants {self.previous_transplants}.')

txmatching/data_transfer_objects/patients/update_dtos/donor_update_dto.py

@@ -3,19 +3,8 @@
 
 from txmatching.data_transfer_objects.patients.update_dtos.patient_update_dto import \
     PatientUpdateDTO
-from txmatching.patients.patient import is_height_valid, is_weight_valid, is_year_of_birth_valid
 
 
 @dataclass
 class DonorUpdateDTO(PatientUpdateDTO):
     active: Optional[bool] = None
-
-    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)

txmatching/data_transfer_objects/patients/update_dtos/patient_update_dto.py

@@ -1,23 +1,21 @@
 from dataclasses import dataclass
 from typing import Optional
 
+from txmatching.data_transfer_objects.patients.patient_base_dto import (
+    PatientBaseDTO, RecipientBaseDTO)
 from txmatching.data_transfer_objects.patients.update_dtos.hla_code_update_dtos import \
     HLATypingUpdateDTO
-from txmatching.patients.patient_parameters import Centimeters, Kilograms
 from txmatching.utils.blood_groups import BloodGroup
 from txmatching.utils.enums import Sex
 
 
-@dataclass
-class PatientUpdateDTO:
+@dataclass(kw_only=True)
+class PatientUpdateDTO(PatientBaseDTO, RecipientBaseDTO):
     # pylint: disable=too-many-instance-attributes
     # It is reasonable to have many attributes here
     db_id: int
     etag: int
     blood_group: Optional[BloodGroup] = None
     hla_typing: Optional[HLATypingUpdateDTO] = None
     sex: Optional[Sex] = None
-    height: Optional[Centimeters] = None
-    weight: Optional[Kilograms] = None
-    year_of_birth: Optional[int] = None
     note: Optional[str] = None

txmatching/data_transfer_objects/patients/update_dtos/recipient_update_dto.py

@@ -3,31 +3,19 @@
 
 from txmatching.data_transfer_objects.patients.hla_antibodies_dto import \
     HLAAntibodiesUpdateDTO
+from txmatching.data_transfer_objects.patients.patient_base_dto import \
+    RecipientBaseDTO
 from txmatching.data_transfer_objects.patients.update_dtos.patient_update_dto import \
     PatientUpdateDTO
-from txmatching.patients.patient import (is_height_valid, is_number_of_previous_transplants_valid,
-    is_weight_valid, is_year_of_birth_valid, RecipientRequirements)
-
+from txmatching.patients.patient import RecipientRequirements
 
 # pylint: disable=too-many-instance-attributes
+
+
 @dataclass
-class RecipientUpdateDTO(PatientUpdateDTO):
+class RecipientUpdateDTO(PatientUpdateDTO, RecipientBaseDTO):
     acceptable_blood_groups: Optional[List[str]] = None
     hla_antibodies: Optional[HLAAntibodiesUpdateDTO] = None
     recipient_requirements: Optional[RecipientRequirements] = None
     cutoff: Optional[int] = None
     waiting_since: Optional[str] = None
-    previous_transplants: Optional[int] = None
-
-    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)

txmatching/data_transfer_objects/patients/upload_dtos/donor_upload_dto.py

@@ -1,14 +1,15 @@
 from dataclasses import dataclass
 from typing import List, Optional
 
-from txmatching.patients.patient import is_height_valid, is_weight_valid, is_year_of_birth_valid
+from txmatching.data_transfer_objects.patients.patient_base_dto import \
+    PatientBaseDTO
 from txmatching.patients.patient_parameters import Centimeters, Kilograms
 from txmatching.utils.blood_groups import BloodGroup
 from txmatching.utils.enums import Sex
 
 
-@dataclass
-class DonorUploadDTO:
+@dataclass(kw_only=True)
+class DonorUploadDTO(PatientBaseDTO):
     # pylint: disable=too-many-instance-attributes
     medical_id: str
     blood_group: BloodGroup
@@ -21,13 +22,3 @@ class DonorUploadDTO:
     year_of_birth: Optional[int] = None
     note: str = ''
     internal_medical_id: Optional[str] = None
-
-    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)

txmatching/data_transfer_objects/patients/upload_dtos/recipient_upload_dto.py

@@ -1,41 +1,23 @@
 from dataclasses import dataclass
 from typing import List, Optional
 
+from txmatching.data_transfer_objects.patients.patient_base_dto import (
+    PatientBaseDTO, RecipientBaseDTO)
 from txmatching.data_transfer_objects.patients.upload_dtos.hla_antibodies_upload_dto import \
     HLAAntibodiesUploadDTO
-from txmatching.patients.patient import (is_height_valid, is_number_of_previous_transplants_valid,
-    is_weight_valid, is_year_of_birth_valid)
-from txmatching.patients.patient_parameters import Centimeters, Kilograms
 from txmatching.utils.blood_groups import BloodGroup
 from txmatching.utils.enums import Sex
 
 
-@dataclass
-class RecipientUploadDTO:
+@dataclass(kw_only=True)
+class RecipientUploadDTO(PatientBaseDTO, RecipientBaseDTO):
     # pylint: disable=too-many-instance-attributes
     acceptable_blood_groups: Optional[List[BloodGroup]]
     medical_id: str
     blood_group: BloodGroup
     hla_typing: List[str]
     hla_antibodies: List[HLAAntibodiesUploadDTO]
     sex: Optional[Sex] = None
-    height: Optional[Centimeters] = None
-    weight: Optional[Kilograms] = None
-    year_of_birth: Optional[int] = None
     note: str = ''
     waiting_since: Optional[str] = None
-    previous_transplants: Optional[int] = None
     internal_medical_id: Optional[str] = None
-
-    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)

txmatching/data_transfer_objects/patients/utils.py

@@ -14,7 +14,8 @@
 
 logger = logging.getLogger(__name__)
 
-
+# is needed here because kw_args in dataclass is not handled well by pylint
+# pylint: disable=unexpected-keyword-arg
 def parsing_issue_to_dto(parsing_issue: ParsingIssue, txm_event: TxmEventBase) -> ParsingIssuePublicDTO:
     return ParsingIssuePublicDTO(
         hla_code_or_group=parsing_issue.hla_code_or_group,

txmatching/patients/hla_code.py

@@ -11,7 +11,7 @@ class HLACode:
     high_res: Optional[str]
     split: Optional[str]
     broad: Optional[str]
-    group: Optional[HLAGroup]
+    group: Optional[HLAGroup] = None
 
     @property
     def display_code(self) -> str:
@@ -24,12 +24,15 @@ def display_code(self) -> str:
         else:
             raise AssertionError('This should never happen. At least one code should be specified.')
 
-    def __init__(self, high_res: Optional[str], split: Optional[str], broad: Optional[str]):
+    def __init__(self, high_res: Optional[str], split: Optional[str], broad: Optional[str], group: HLAGroup = None):
         assert high_res is not None or broad is not None
         self.high_res = high_res
         self.split = split
         self.broad = broad
-        self.group = self.group_from_hla_code
+        if group:
+            self.group = group
+        else:
+            self.group = self.group_from_hla_code
 
     def __repr__(self):
         return f'HLACode({repr(self.high_res)}, {repr(self.split)}, {repr(self.broad)})'

txmatching/patients/patient.py

@@ -1,13 +1,11 @@
 from dataclasses import dataclass
-from datetime import date, datetime
+from datetime import datetime
 from enum import Enum
 from typing import Dict, List, Optional, Tuple
 
-from txmatching.auth.exceptions import InvalidArgumentException
 from txmatching.data_transfer_objects.hla.parsing_issue_dto import ParsingIssue
 from txmatching.patients.hla_model import HLAAntibodies, HLAAntibodyRaw
-from txmatching.patients.patient_parameters import (Centimeters, Kilograms,
-                                                    PatientParameters)
+from txmatching.patients.patient_parameters import PatientParameters
 from txmatching.patients.patient_types import DonorDbId, RecipientDbId
 from txmatching.utils.blood_groups import BloodGroup
 from txmatching.utils.enums import TxmEventState
@@ -17,7 +15,6 @@
                                               update_persistent_hash)
 
 DEFAULT_CUTOFF = 2000
-THIS_YEAR = date.today().year
 
 
 class DonorType(str, Enum):
@@ -62,6 +59,7 @@ def update_persistent_hash(self, hash_: HashType):
         update_persistent_hash(hash_, self.donor_type)
         update_persistent_hash(hash_, self.active)
 
+
 # pylint: disable=too-many-instance-attributes
 @dataclass
 class RecipientRequirements(PersistentlyHashable):
@@ -172,27 +170,6 @@ def calculate_cutoff(hla_antibodies_raw_list: List[HLAAntibodyRaw]) -> int:
                )).cutoff
 
 
-def is_height_valid(patient: str, height: Centimeters):
-    if height < 0:
-        raise InvalidArgumentException(f'Invalid {patient} height {height}cm.')
-
-
-def is_weight_valid(patient: str, weight: Kilograms):
-    if weight < 0:
-        raise InvalidArgumentException(f'Invalid {patient} weight {weight}kg.')
-
-
-def is_year_of_birth_valid(patient: str, year_of_birth: Centimeters):
-    if year_of_birth < 1900 or year_of_birth > THIS_YEAR:
-        raise InvalidArgumentException(f'Invalid {patient} year of birth {year_of_birth}')
-
-
-def is_number_of_previous_transplants_valid(previous_transplants: int):
-    if previous_transplants and previous_transplants < 0:
-        raise InvalidArgumentException(
-            f'Invalid recipient number of previous transplants {previous_transplants}.')
-
-
 def _filter_patients_that_dont_have_parsing_errors_or_unconfirmed_warnings(
         donors: List[Donor], recipients: List[Recipient]
 ) -> Tuple[Dict[DonorDbId, Donor], Dict[RecipientDbId, Recipient]]:

txmatching/patients/patient_parameters.py

@@ -1,16 +1,15 @@
 from dataclasses import dataclass
 from typing import Optional
 
+from txmatching.data_transfer_objects.patients.patient_base_dto import (
+    Centimeters, Kilograms)
 from txmatching.patients.hla_model import HLATyping
 from txmatching.utils.blood_groups import BloodGroup
 from txmatching.utils.country_enum import Country
 from txmatching.utils.enums import Sex
 from txmatching.utils.persistent_hash import (HashType, PersistentlyHashable,
                                               update_persistent_hash)
 
-Kilograms = float
-Centimeters = int
-
 
 # It make sense to have a lot of patient parameters
 # pylint: disable=too-many-instance-attributes

txmatching/solve_service/solver_lock.py

@@ -17,7 +17,7 @@ def run_with_solver_lock(block: Callable[[], T]) -> T:
     Runs execution with the lock, if some other execution is running right now,
     it raises SolverAlreadyRunningException.
     """
-    # pylint: disable=global-statement
+    # pylint: disable=global-statement,global-variable-not-assigned
     global _solver_running_lock, _solver_running
     try:
         # check whether another solver is running

txmatching/utils/excel_parsing/parse_excel_data.py

@@ -35,8 +35,8 @@ class ExcelSource(str, Enum):
     IKEM = 'IKEM'
     BEL_2 = 'BEL_2'
 
-
-# pylint: disable=too-many-instance-attributes
+# is needed here because kw_args in dataclass is not handled well by pylint
+# pylint: disable=too-many-instance-attributes,unexpected-keyword-arg
 @dataclass
 class ExcelColumnsMap:
     donor_id: str