Improving readability/consistency of errors/exceptions in gempyor (second batch) #387

emprzy · 2024-11-06T19:32:53Z

Describe your changes.

This pull request adjusts syntax, clarity, and formatting of errors/exceptions in gempyor's model_info.py, outcomes.py, seeding.py, seir.py, and simulation_component.py. This is the second of many sequential PR's to address the lack of consistency in formatting and style in gempyor errors/exceptions.

This pull request does NOT attempt to narrow or refine which type of errors are thrown, nor does it address the poor practice of using print() statements (as opposed to a logger) for stray warnings or errors. See GH- #310.

What does your pull request address? Tag relevant issues.

This pull request addresses GH- #281 .

By splitting this up into multiple PR's, hopefully it will make it easier to review, and also make it easier for me to be dynamic in the solutions I implement if anyone has any suggestions for the way I am doing this.

Includes model_info.py, outcomes.py, seeding.py, seir.py, and simulation_component.py (this last one didn't require any edits).

emprzy · 2024-11-06T19:33:29Z

Creating this PR so that I can easily see the conflicts that need to be resolved. This is not ready for review yet!

pearsonca · 2024-11-06T20:39:05Z

Creating this PR so that I can easily see the conflicts that need to be resolved. This is not ready for review yet!

Fine for this go round, but consider using the "draft PR" functionality in the future.

emprzy · 2024-11-08T14:13:49Z

Creating this PR so that I can easily see the conflicts that need to be resolved. This is not ready for review yet!

Fine for this go round, but consider using the "draft PR" functionality in the future.

Ah, had no idea this functionality existed. Great, thank you!!

TimothyWillard · 2024-11-08T20:03:56Z

Can we modify this PR to merge into the dev branch instead of main? I don't see any breaking changes for main at a glance, but I would like to move us towards merging feature branches into the dev branch. I think this will be a great addition to the December release!

TimothyWillard

I tried to make all of the quick things suggestions to make it a breeze to get through everything this time. Like before, mostly nit-picky things relating to shortening messages and removing awkward punctuation, so hopefully quick to get through. One thing that is not in the code is this warning that came from the CI:

=============================== warnings summary ===============================
tests/parameters/test_parameters_class.py:286
tests/parameters/test_parameters_class.py:286
  /home/runner/work/flepiMoP/flepiMoP/flepimop/gempyor_pkg/tests/parameters/test_parameters_class.py:286: DeprecationWarning: invalid escape sequence '\:'
    f"Provided file dates span '{timeseries_start_date}( 00\:00\:00)?' to '{timeseries_end_date}( 00\:00\:00)?', "

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================= 410 passed, 2 warnings in 217.92s (0:03:37) ==================

This may have snuck in with GH-313, but can we fix here? See: https://github.com/HopkinsIDD/flepiMoP/actions/runs/11748501992/job/32732711221.

flepimop/gempyor_pkg/src/gempyor/model_info.py

flepimop/gempyor_pkg/src/gempyor/seir.py

flepimop/gempyor_pkg/tests/seir/test_seir.py

flepimop/gempyor_pkg/src/gempyor/model_info.py

Co-authored-by: Timothy Willard <[email protected]>

the function `test_raises_not_implemented_error()` in test file `test_read_df.py` on account that it was receiving a `ValueError` when it expected a `NotImplementedError`. I switched the error within `utils.py:read_df()` to be an expected `NotImplementedError`.

There were not that many changes in the final batch of error improvements, so I went ahead and lumped them in with batch two.

emprzy · 2024-11-20T18:16:35Z

Progress update: there were not that many changes in the final batch of error improvements, so I went ahead and put them in this PR.

pearsonca

Seems generally good, but a few errors seem to have crept in.

flepimop/gempyor_pkg/src/gempyor/model_info.py

pearsonca · 2024-11-20T22:00:27Z

flepimop/gempyor_pkg/tests/seir/test_model_info.py

@@ -44,7 +44,10 @@ def test_ModelInfo_init_tf_is_ahead_of_ti_fail(self):
        config.read(user=False)
        config.set_file(f"{DATA_DIR}/config_test.yml")
        config["start_date"] = "2022-01-02"
-        with pytest.raises(ValueError, match=r"tf.*is less than or equal to ti.*"):
+        with pytest.raises(


I'd prefer this match to the error-causing values (to verify that the error contains the offending values for the user to see), rather than the precise wording of the message.

Note to self: need to find how ti and tf are referenced in this file

pearsonca · 2024-11-20T22:01:26Z

flepimop/gempyor_pkg/tests/statistics/test_statistic_class.py

-        with pytest.raises(
-            ValueError, match=rf"^Unsupported regularization\: {unsupported_name}$"
-        ):
+        with pytest.raises(ValueError, match=r"^Unsupported regularization \[received:.*$"):


Again, prefer this matches the offending value offered rather than the precise message.

pearsonca · 2024-11-20T22:01:58Z

flepimop/gempyor_pkg/tests/statistics/test_statistic_class.py

-            rf"the same shape\: model\_data\.shape\=\({model_rows}\, {model_cols}\) "
-            rf"\!\= gt\_data\.shape\=\({gt_rows}\, {gt_cols}\)$"
-        )
+        expected_match = rf".*do not have the same shape:.*"


ibid, though a bit more complicated.

pearsonca · 2024-11-20T22:02:32Z

flepimop/gempyor_pkg/tests/utils/test_random_distribution_sampler.py

@@ -14,15 +14,15 @@ class TestRandomDistributionSampler:
    def test_not_implemented_error_exception(self, distribution: str) -> None:
        with pytest.raises(
            NotImplementedError,
-            match=rf"^unknown distribution \[got\: {distribution}\]$",
+            match=rf"^Unknown distribution \[received.*",


prefer match bad argument value rather than message

pearsonca · 2024-11-20T22:03:23Z

flepimop/gempyor_pkg/tests/utils/test_read_df.py

@@ -38,13 +38,13 @@ def test_raises_not_implemented_error(self) -> None:
        """
        with pytest.raises(
            expected_exception=NotImplementedError,
-            match="Invalid extension txt. Must be 'csv' or 'parquet'.",
+            match=r".*Supported extensions are `.csv` or `.parquet`.",


prefer match bad argument value rather than message (though also worth matching the supported argument values, to confirm we're informing users).

(this comment repeats for a few more changes here and in test_utils)

Awaiting regex error kickback to adjust the test files.

emprzy and others added 2 commits November 1, 2024 11:26

Improving errors in another set of gempyor files

f67d3bc

Includes model_info.py, outcomes.py, seeding.py, seir.py, and simulation_component.py (this last one didn't require any edits).

Merge branch 'HopkinsIDD:main' into gempyor_error_improvments_part2

141a627

emprzy and others added 5 commits November 8, 2024 12:40

Merge branch 'main' into gempyor_error_improvements_part2

1fe2d4b

Applied black linting to this branch

90b3618

Merge branch 'main' into gempyor_error_improvements_part2

1934338

Updating a RegEx in test_model_info.py

a6c3585

black linting and RegEx alteration

e2a87c3

emprzy requested review from TimothyWillard, pearsonca and jcblemai November 8, 2024 19:47

emprzy changed the base branch from main to dev November 8, 2024 20:15

TimothyWillard requested changes Nov 11, 2024

View reviewed changes

emprzy and others added 6 commits November 13, 2024 11:34

Update flepimop/gempyor_pkg/src/gempyor/seir.py

17a4f38

Co-authored-by: Timothy Willard <[email protected]>

Update flepimop/gempyor_pkg/src/gempyor/seir.py

3acad12

Co-authored-by: Timothy Willard <[email protected]>

Implementing Tim's suggestions!

44ccc84

Regex adjustment

b32c64e

Regex update

d844ced

Implementing Tim's suggestions

2cbf2ed

TimothyWillard assigned emprzy Nov 18, 2024

emprzy added 7 commits November 18, 2024 13:03

Regex update in test_statistic_class.py

a1692a1

Regex update in test_statistic_class.py

759795f

Regex adjusment in test_random_distribution_sampler.py

df784d5

Fixing typo in utils.py

8ec6c05

Regex update in test_random_distribution_sampler.py

d760ddc

Regex update in test_random_distribution_sampler.py

7e19700

Update utils.py

452ddbb

the function `test_raises_not_implemented_error()` in test file `test_read_df.py` on account that it was receiving a `ValueError` when it expected a `NotImplementedError`. I switched the error within `utils.py:read_df()` to be an expected `NotImplementedError`.

emprzy added 8 commits November 20, 2024 10:42

Regex update in test_read_df.py

99d2f36

Regex update in test_read_df.py

47514d5

Regex update in test_read_df.py

97d3996

Updating a unit test in test_utils.py

918ebc7

Regex update in test_utils.py and error type agreement with utils.py

764c4b2

Regex update in test_write_df.py

c1e79c2

Regex update in test-write_df.py

4fd1b11

Removing deprecated style of \ to escape colons

2e62ae3

emprzy requested a review from TimothyWillard November 20, 2024 17:46

emprzy added 2 commits November 20, 2024 12:56

linting with black

e5772c7

Updating steps_source.py and utils.py

8fe918a

There were not that many changes in the final batch of error improvements, so I went ahead and lumped them in with batch two.

jcblemai approved these changes Nov 20, 2024

View reviewed changes

pearsonca requested changes Nov 20, 2024

View reviewed changes

emprzy added 4 commits November 22, 2024 14:35

Implementing Carl's suggestions

f7e53a7

Awaiting regex error kickback to adjust the test files.

Regex update in test_model_info.py

d5fe360

Regex update in test_model_info.py

9c6e1f1

Regex update in test_model_info.py

f7c1ad0

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improving readability/consistency of errors/exceptions in gempyor (second batch) #387

Improving readability/consistency of errors/exceptions in gempyor (second batch) #387

emprzy commented Nov 6, 2024

emprzy commented Nov 6, 2024

pearsonca commented Nov 6, 2024

emprzy commented Nov 8, 2024

TimothyWillard commented Nov 8, 2024

TimothyWillard left a comment

emprzy commented Nov 20, 2024

pearsonca left a comment

pearsonca Nov 20, 2024

emprzy Nov 22, 2024

pearsonca Nov 20, 2024

pearsonca Nov 20, 2024

pearsonca Nov 20, 2024

pearsonca Nov 20, 2024

pearsonca Nov 20, 2024

Improving readability/consistency of errors/exceptions in gempyor (second batch) #387

Are you sure you want to change the base?

Improving readability/consistency of errors/exceptions in gempyor (second batch) #387

Conversation

emprzy commented Nov 6, 2024

Describe your changes.

What does your pull request address? Tag relevant issues.

emprzy commented Nov 6, 2024

pearsonca commented Nov 6, 2024

emprzy commented Nov 8, 2024

TimothyWillard commented Nov 8, 2024

TimothyWillard left a comment

Choose a reason for hiding this comment

emprzy commented Nov 20, 2024

pearsonca left a comment

Choose a reason for hiding this comment

pearsonca Nov 20, 2024

Choose a reason for hiding this comment

emprzy Nov 22, 2024

Choose a reason for hiding this comment

pearsonca Nov 20, 2024

Choose a reason for hiding this comment

pearsonca Nov 20, 2024

Choose a reason for hiding this comment

pearsonca Nov 20, 2024

Choose a reason for hiding this comment

pearsonca Nov 20, 2024

Choose a reason for hiding this comment

pearsonca Nov 20, 2024

Choose a reason for hiding this comment