biosimulators / Biosimulators_COPASI

COPASI biochemical network simulation program via BioSimulators-compliant command-line interface and Docker container
https://docs.biosimulators.org/Biosimulators_COPASI
MIT License
2 stars 3 forks source link

Lowering Cognitive complexity as per SonarCloud #58

Open AlexPatrie opened 1 year ago

AlexPatrie commented 1 year ago

Justification After several attempts of pushing type annotations to biosimulators_copasi with a PR, I have been repeatedly met with an error from SonarCloud which indicates that a given function’s “cognitive complexity” is above the acceptable threshold. The documentation specifies that it will assess a negative mark for a variety of statements and situations including: nesting, recursion, and any statement that includes if/else/while/except/and/or/is/not. Exceptions to this rule are made with try/finally blocks and boolean dictionaries.

Examples: For example, I was able to devise a solution that satisfied the bot while still returning the desired result for the optional keyword argument “config”, where the default is None:

Originally: (+1 cognitive complexity) config = config or get_config()

Revised: (+0 cognitive complexity) config = {True: config, False: get_config()}.get(bool(config), config)

Clearly, this solution is neither scalable nor ‘Pythonic’ in nature. Another source of SonarCloud contention pertains to nested-if statements whose depth of nesting is > 1. For example:

` for change in model.changes: target_sbml_id = model_change_target_sbml_id_map[change.target] copasi_model_obj = get_copasi_model_object_by_sbml_id(copasi_model, target_sbml_id, units) if copasi_model_obj is None: invalid_changes.append(change.target) else: model_obj_parent = copasi_model_obj.getObjectParent()

              if isinstance(model_obj_parent, COPASI.CCompartment): 
                  set_func = model_obj_parent.setInitialValue
                  ref = model_obj_parent.getInitialValueReference()

              elif isinstance(model_obj_parent, COPASI.CModelValue): 
                  set_func = model_obj_parent.setInitialValue
                  ref = model_obj_parent.getInitialValueReference()

              elif isinstance(model_obj_parent, COPASI.CMetab): 
                  if units == Units.discrete: 
                      set_func = model_obj_parent.setInitialValue
                      ref = model_obj_parent.getInitialValueReference()
                  else:
                      set_func = model_obj_parent.setInitialConcentration
                      ref = model_obj_parent.getInitialConcentrationReference()

              model_change_obj_map[change.target] = (set_func, ref)`

Possible solutions: One effective solution to this could include a guard clause. The proposal at hand involves refactoring nested conditional statements whose traversal depth is > 1. The above code is a perfect example of this cognitive complexity. If setup required python >= 3.10, then the implementation of match/case statements in place of ternary-if statements would satisfy the requirements of this issue.