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

Improving readability/consistency of errors/exceptions in gempyor. #313

Merged
merged 53 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
53 commits
Select commit Hold shift + click to select a range
452902f
Update parameters.py
emprzy Aug 15, 2024
9a7f4a3
Update parameters.py
emprzy Aug 15, 2024
c37a605
Merge branch 'HopkinsIDD:main' into gempyor_error_improvements
emprzy Sep 11, 2024
d063762
Improved readability/consistency of errors/exceptions in 5 gempyor fi…
emprzy Sep 16, 2024
0cdeed1
Merge branch 'HopkinsIDD:main' into gempyor_error_improvements
emprzy Sep 20, 2024
add67d1
Attempting to fix errors by changing the regexs in some of the test f…
emprzy Sep 20, 2024
3b4e490
Another attempt to fix RegExs in test files
emprzy Sep 27, 2024
e3df5d8
Update initial_conditions.py
emprzy Oct 2, 2024
2066ee4
Update parameters.py
emprzy Oct 2, 2024
2b3feed
Update flepimop/gempyor_pkg/tests/parameters/test_parameters_class.py
emprzy Oct 4, 2024
72aaa84
Small adjustments to compartments.py
emprzy Oct 4, 2024
4639721
Some more small syntactical adjustments.
emprzy Oct 4, 2024
064ec7e
Update flepimop/gempyor_pkg/src/gempyor/compartments.py
emprzy Oct 14, 2024
f614127
Continuing to evolve error/exception messags
emprzy Oct 16, 2024
9c20318
Merge branch 'gempyor_error_improvements' of https://github.com/emprz…
emprzy Oct 16, 2024
3b20e8a
Fixing reg-exs
emprzy Oct 16, 2024
afee285
Updating RegEx
emprzy Oct 16, 2024
2eaa427
Fixing RegExs again UGH!
emprzy Oct 16, 2024
48a78ee
I'm losing my mind with this RegEx
emprzy Oct 16, 2024
6e1410f
Update test_parameters_class.py
emprzy Oct 16, 2024
5c09a3c
Update flepimop/gempyor_pkg/tests/parameters/test_parameters_class.py
emprzy Oct 16, 2024
2785437
Shortening a verbose error
emprzy Oct 16, 2024
4cc651f
RegEx update
emprzy Oct 16, 2024
be85d72
Regex edit..
emprzy Oct 18, 2024
fb549c4
Regex edit
emprzy Oct 18, 2024
135aa54
Update flepimop/gempyor_pkg/src/gempyor/compartments.py
emprzy Oct 18, 2024
681ff79
Update flepimop/gempyor_pkg/src/gempyor/compartments.py
emprzy Oct 18, 2024
1549b5b
Update flepimop/gempyor_pkg/src/gempyor/compartments.py
emprzy Oct 18, 2024
cbfd093
Update flepimop/gempyor_pkg/src/gempyor/compartments.py
emprzy Oct 18, 2024
ccfb6e0
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
e83f250
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
6eed1e3
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
08e25a3
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
e68dc8a
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
960f4d2
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
2b75d6a
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
5d999dd
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
a15184e
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
2b2a281
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
76b5adf
Update flepimop/gempyor_pkg/src/gempyor/initial_conditions.py
emprzy Oct 18, 2024
a0ae5e3
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
0c14d66
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
9a6ba69
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
4ca01af
Update flepimop/gempyor_pkg/src/gempyor/config_validator.py
emprzy Oct 18, 2024
88e310d
Update flepimop/gempyor_pkg/src/gempyor/inference.py
emprzy Oct 18, 2024
01d11a7
Update parameters.py and test_parameters_class.py
emprzy Oct 18, 2024
fd049dd
Merge branch 'HopkinsIDD:main' into gempyor_error_improvements
emprzy Nov 4, 2024
7096ef1
Updated compartments.py, config_validaor.py, inference.py, parameters…
emprzy Nov 4, 2024
ef62ec3
Merge branch 'main' into gempyor_error_improvements
emprzy Nov 6, 2024
99b4aec
Update parameters.py
emprzy Nov 6, 2024
d2f8cc2
Applying `black` to this branch
emprzy Nov 6, 2024
cde6acb
linting my code with `black`
emprzy Nov 8, 2024
7b4f6e8
Capitalization correct in a RegEx in `test_ic.py`
emprzy Nov 8, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
90 changes: 46 additions & 44 deletions flepimop/gempyor_pkg/src/gempyor/compartments.py
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ def __init__(
self.times_set += 1

if self.times_set == 0:
raise ValueError("Compartments object not set, no config or file provided")
raise ValueError("Compartments object not set, no config or file provided.")
return

def constructFromConfig(self, seir_config, compartment_config):
Expand Down Expand Up @@ -153,15 +153,15 @@ def expand_transition_elements(self, single_transition_config, problem_dimension
except Exception as e:
print(f"Error {e}:")
print(
f">>> in expand_transition_elements for `source:` at index {it.multi_index}"
f">>> in expand_transition_elements for `source:` at index '{it.multi_index}'"
)
print(
f">>> this transition source is: {single_transition_config['source']}"
f">>> this transition source is: '{single_transition_config['source']}'"
)
print(
f">>> this transition destination is: {single_transition_config['destination']}"
f">>> this transition destination is: '{single_transition_config['destination']}'"
)
print(f"transition_dimension: {problem_dimension}")
print(f"transition_dimension: '{problem_dimension}'")
raise e

try:
Expand All @@ -175,15 +175,15 @@ def expand_transition_elements(self, single_transition_config, problem_dimension
except Exception as e:
print(f"Error {e}:")
print(
f">>> in expand_transition_elements for `destination:` at index {it.multi_index}"
f">>> in expand_transition_elements for `destination:` at index '{it.multi_index}'"
)
print(
f">>> this transition source is: {single_transition_config['source']}"
f">>> this transition source is: '{single_transition_config['source']}'"
)
print(
f">>> this transition destination is: {single_transition_config['destination']}"
f">>> this transition destination is: '{single_transition_config['destination']}'"
)
print(f"transition_dimension: {problem_dimension}")
print(f"transition_dimension: '{problem_dimension}'")
raise e

try:
Expand All @@ -197,15 +197,15 @@ def expand_transition_elements(self, single_transition_config, problem_dimension
except Exception as e:
print(f"Error {e}:")
print(
f">>> in expand_transition_elements for `rate:` at index {it.multi_index}"
f">>> in expand_transition_elements for `rate:` at index '{it.multi_index}'"
)
print(
f">>> this transition source is: {single_transition_config['source']}"
f">>> this transition source is: '{single_transition_config['source']}'"
)
print(
f">>> this transition destination is: {single_transition_config['destination']}"
f">>> this transition destination is: '{single_transition_config['destination']}'"
)
print(f"transition_dimension: {problem_dimension}")
print(f"transition_dimension: '{problem_dimension}'")
raise e

try:
Expand All @@ -225,15 +225,15 @@ def expand_transition_elements(self, single_transition_config, problem_dimension
except Exception as e:
print(f"Error {e}:")
print(
f">>> in expand_transition_elements for `proportional_to:` at index {it.multi_index}"
f">>> in expand_transition_elements for `proportional_to:` at index '{it.multi_index}'"
)
print(
f">>> this transition source is: {single_transition_config['source']}"
f">>> this transition source is: '{single_transition_config['source']}'"
)
print(
f">>> this transition destination is: {single_transition_config['destination']}"
f">>> this transition destination is: '{single_transition_config['destination']}'"
)
print(f"transition_dimension: {problem_dimension}")
print(f"transition_dimension: '{problem_dimension}'")
raise e

if (
Expand All @@ -260,15 +260,15 @@ def expand_transition_elements(self, single_transition_config, problem_dimension
except Exception as e:
print(f"Error {e}:")
print(
f">>> in expand_transition_elements for `proportion_exponent:` at index {it.multi_index}"
f">>> in expand_transition_elements for `proportion_exponent:` at index '{it.multi_index}'"
)
print(
f">>> this transition source is: {single_transition_config['source']}"
f">>> this transition source is: '{single_transition_config['source']}'"
)
print(
f">>> this transition destination is: {single_transition_config['destination']}"
f">>> this transition destination is: '{single_transition_config['destination']}'"
)
print(f"transition_dimension: {problem_dimension}")
print(f"transition_dimension: '{problem_dimension}'")
raise e
else:
new_transition_config["proportion_exponent"][it.multi_index] = [
Expand Down Expand Up @@ -379,7 +379,6 @@ def unformat_proportion_exponent(
def parse_single_transition(
self, seir_config, single_transition_config, fake_config=False
):

## This method relies on having run parse_compartments
if not fake_config:
single_transition_config = single_transition_config.get()
Expand Down Expand Up @@ -483,7 +482,9 @@ def get_comp_idx(self, comp_dict: dict, error_info: str = "no information") -> i
comp_idx = self.compartments[mask].index.values
if len(comp_idx) != 1:
raise ValueError(
f"The provided dictionary does not allow to isolate a compartment: {comp_dict} isolate {self.compartments[mask]} from options {self.compartments}. The get_comp_idx function was called by'{error_info}'."
f"The provided dictionary does not allow an isolated compartment: '{comp_dict}'. "
f"Isolate '{self.compartments[mask]}'. "
f"The `get_comp_idx` function was called by '{error_info}'."
)
return comp_idx[0]

Expand All @@ -503,9 +504,8 @@ def get_transition_array(self):
if self.compartments["name"][compartment] == elem:
rc = compartment
if rc == -1:
print(self.compartments)
raise ValueError(
f"Could not find {colname} defined by {elem} in compartments"
f"Could not find '{colname}' defined by '{elem}' in '{self.compartments}'."
)
transition_array[cit, it] = rc

Expand Down Expand Up @@ -539,8 +539,10 @@ def get_transition_array(self):
candidate = reduce(lambda a, b: a + "*" + b, elem)
candidate = candidate.replace(" ", "")
# candidate = candidate.replace("*1", "")
if not candidate in unique_strings:
raise ValueError("Something went wrong")
if candidate not in unique_strings:
raise ValueError(
f"Candidate '{candidate}' from 'rate' column is not in the list of unique strings: {unique_strings}."
)
rc = [it for it, x in enumerate(unique_strings) if x == candidate][0]
transition_array[2][it] = rc

Expand Down Expand Up @@ -583,8 +585,10 @@ def get_transition_array(self):
candidate = reduce(lambda a, b: a + "*" + b, y)
candidate = candidate.replace(" ", "")
# candidate = candidate.replace("*1", "")
if not candidate in unique_strings:
raise ValueError("Something went wrong")
if candidate not in unique_strings:
raise ValueError(
f"Proportion exponent '{candidate}' is not found in the list of unique strings: '{unique_strings}'."
)
rc = [it for it, x in enumerate(unique_strings) if x == candidate][0]
proportion_info[2][proportion_compartment_index] = rc
proportion_compartment_index += 1
Expand All @@ -610,7 +614,8 @@ def get_transition_array(self):
rc = compartment
if rc == -1:
raise ValueError(
f"Could not find proportional_to {elem3} in compartments"
f"Could not find `proportional_to` '{elem3}' in compartments. "
f"Available compartments: '{self.compartments}'."
)

proportion_array[proportion_index] = rc
Expand Down Expand Up @@ -682,7 +687,7 @@ def parse_parameter_strings_to_numpy_arrays_v2(
parsed_formulas.append(f)
except Exception as e:
print(
f"Cannot parse formula: '{formula}' from parameters {parameter_names}"
f"Cannot parse formula '{formula}' from parameters: '{parameter_names}'."
)
raise (e) # Print the error message for debugging

Expand Down Expand Up @@ -733,10 +738,10 @@ def parse_parameter_strings_to_numpy_arrays(
not operators
): # empty list means all have been tried. Usually there just remains one string in string_list at that time.
raise ValueError(
f"""Could not parse string {string_list}.
This usually mean that '{string_list[0]}' is a parameter name that is not defined
or that it contains an operator that is not in the list of supported operator: ^,*,/,+,-.
The defined parameters are {parameter_names}."""
f"Could not parse string '{string_list}'. "
f"This usually means that '{string_list[0]}' is a parameter name that is not defined "
f"or that it contains an operator that is not in the list of supported operators: '{operators}'. "
f"The defined parameters are '{parameter_names}'."
)

split_strings = [x.split(operators[0]) for x in string_list]
Expand Down Expand Up @@ -851,16 +856,13 @@ def list_access_element_safe(thing, idx, dimension=None, encapsulate_as_list=Fal
try:
return list_access_element(thing, idx, dimension, encapsulate_as_list)
except Exception as e:
print(f"Error {e}:")
print(f">>> in list_access_element_safe for {thing} at index {idx}")
print(
">>> This is often, but not always because the object above is a list (there are brackets around it)."
)
print(
">>> and in this case it is not broadcast, so if you want to it to be broadcasted, you need remove the brackets around it."
raise Exception(
f"Error {e}: "
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Saying "error" in an error message is redundant, and also unclear in this context. Maybe something like "Raised exception: e"?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

n.b., there are some other case to apply this style to.

f"in list_access_element_safe for '{thing}' at index '{idx}'. "
f"This is often, but not always because the object above is a list (there are brackets around it). "
f"and in this case it is not broadcast, so if you want to it to be broadcasted, you need remove the brackets around it. "
f"dimension: '{dimension}'."
)
print(f"dimension: {dimension}")
raise e


def list_access_element(thing, idx, dimension=None, encapsulate_as_list=False):
Expand Down
55 changes: 25 additions & 30 deletions flepimop/gempyor_pkg/src/gempyor/config_validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -70,14 +70,14 @@ def validate_initial_file_check(cls, values):
initial_file_type = values.get("initial_file_type")
if method in {"FromFile", "SetInitialConditions"} and not initial_conditions_file:
raise ValueError(
f"Error in InitialConditions: An initial_conditions_file is required when method is {method}"
f"An `initial_conditions_file` is required when method is '{method}'."
)
if (
method in {"InitialConditionsFolderDraw", "SetInitialConditionsFolderDraw"}
and not initial_file_type
):
raise ValueError(
f"Error in InitialConditions: initial_file_type is required when method is {method}"
f"An `initial_file_type` is required when method is '{method}'."
)
return values

Expand All @@ -86,9 +86,7 @@ def plugin_filecheck(cls, values):
method = values.get("method")
plugin_file_path = values.get("plugin_file_path")
if method == "plugin" and not plugin_file_path:
raise ValueError(
"Error in InitialConditions: a plugin file path is required when method is plugin"
)
raise ValueError("A plugin file path is required when method is plugin.")
return values


Expand Down Expand Up @@ -121,16 +119,14 @@ def validate_seedingfile(cls, values):
seeding_file = values.get("seeding_file")
if method == "PoissonDistributed" and not lambda_file:
raise ValueError(
f"Error in Seeding: A lambda_file is required when method is {method}"
"A `lambda_file` is required when method is 'PoissonDistributed'."
)
if method == "FolderDraw" and not seeding_file_type:
raise ValueError(
"Error in Seeding: A seeding_file_type is required when method is FolderDraw"
"A `seeding_file_type` is required when method is 'FolderDraw'."
)
if method == "FromFile" and not seeding_file:
raise ValueError(
"Error in Seeding: A seeding_file is required when method is FromFile"
)
raise ValueError("A `seeding_file` is required when method is 'FromFile'.")
return values

@model_validator(mode="before")
Expand All @@ -139,7 +135,8 @@ def plugin_filecheck(cls, values):
plugin_file_path = values.get("plugin_file_path")
if method == "plugin" and not plugin_file_path:
raise ValueError(
"Error in Seeding: a plugin file path is required when method is plugin"
f"A plugin file path is required when method is 'plugin', "
f"was given '{plugin_file_path!r}'."
)
return values

Expand Down Expand Up @@ -173,16 +170,14 @@ def check_distr(cls, values):
if distr != "fixed":
if not mean and not sd:
raise ValueError(
"Error in value: mean and sd must be provided for non-fixed distributions"
"Mean and sd must be provided for non-fixed distributions."
)
if distr == "truncnorm" and not a and not b:
raise ValueError(
"Error in value: a and b must be provided for truncated normal distributions"
"a and b must be provided for truncated normal distributions."
)
if distr == "fixed" and not value:
raise ValueError(
"Error in value: value must be provided for fixed distributions"
)
raise ValueError("Value must be provided for fixed distributions.")
return values


Expand Down Expand Up @@ -211,7 +206,7 @@ def which_value(cls, values):
timeseries = values.get("timeseries") is not None
if value and timeseries:
raise ValueError(
"Error in seir::parameters: your parameter is both a timeseries and a value, please choose one"
f"Parsed a parameter with both a timeseries, '{values.get('timeseries')!r}', and a value, '{values.get('value')!r}', please choose one."
)
return values

Expand Down Expand Up @@ -301,7 +296,7 @@ def which_source(cls, values):
prevalence = values.get("prevalence")
if incidence and prevalence:
raise ValueError(
emprzy marked this conversation as resolved.
Show resolved Hide resolved
"Error in outcomes::source. Can only be incidence or prevalence, not both."
f"Parsed a source with both an incidence, '{values.get('incidence')!r}', and prevalence, '{values.get('prevalence')!r}', please choose one."
)
return values

Expand Down Expand Up @@ -339,13 +334,9 @@ def check_outcome_type(cls, values):
source_present = values.get("source") is not None

if sum_present and source_present:
raise ValueError(
f"Error in outcome: Both 'sum' and 'source' are present. Choose one."
)
raise ValueError("Both 'sum' and 'source' are present, please choose one.")
elif not sum_present and not source_present:
raise ValueError(
f"Error in outcome: Neither 'sum' nor 'source' is present. Choose one."
)
raise ValueError("Neither 'sum' nor 'source' is present, please choose one.")
return values


Expand All @@ -364,7 +355,7 @@ def check_paramfromfile_type(cls, values):

if param_from_file and not param_subpop_file:
raise ValueError(
f"Error in outcome: 'param_subpop_file' is required when 'param_from_file' is True"
"A `param_subpop_file` is required when `param_from_file` is 'True'."
)
return values

Expand Down Expand Up @@ -450,11 +441,11 @@ def verify_inference(cls, values):
start_date_groundtruth = values.get("start_date_groundtruth") is not None
if inference_present and not start_date_groundtruth:
raise ValueError(
"Inference mode is enabled but no groundtruth dates are provided"
"Inference mode is enabled, but no groundtruth dates are provided. Please provide groundtruth dates."
)
elif start_date_groundtruth and not inference_present:
raise ValueError(
"Groundtruth dates are provided but inference mode is not enabled"
"Groundtruth dates are provided, but inference mode is not enabled. Please enable inference mode."
)
return values

Expand All @@ -464,13 +455,17 @@ def check_dates(cls, values):
end_date = values.get("end_date")
if start_date and end_date:
if end_date <= start_date:
raise ValueError("end_date must be greater than start_date")
raise ValueError(
f"`end_date` ('{end_date}') must be later than `start_date` ('{start_date}'))."
)
return values

@model_validator(mode="before")
def init_or_seed(cls, values):
init = values.get("initial_conditions")
seed = values.get("seeding")
if not init or seed:
raise ValueError("either initial_conditions or seeding must be provided")
if not (init or seed):
raise ValueError(
f"At least one of `initial_conditions` or `seeding` must be provided."
)
return values
Loading