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

Checking an error is raised by a test #108

Open
p-j-smith opened this issue Jan 13, 2022 · 2 comments · May be fixed by #451
Open

Checking an error is raised by a test #108

p-j-smith opened this issue Jan 13, 2022 · 2 comments · May be fixed by #451
Assignees

Comments

@p-j-smith
Copy link

In the Defensive Programming section, there is a test for patient_normalise that checks a ValueError is raised when negative values are passed to the function:

with pytest.raises(raises):
    npt.assert_almost_equal(patient_normalise(np.array(test)), np.array(expected), decimal=2)

This could be simplified to:

with pytest.raises(raises):
    patient_normalise(np.asarray(test))

Also, it's best to check the specific error message being thrown:

with pytest.raises(raises, match='Inflammation values should not be negative'):
    patient_normalise(np.asarray(test))

And there is a check for whether raises is ValueError or None:

if raises:

However, it's better to always explicitly check that a variable is not None:

if raises is not None:

It makes no difference here, but other times some statements (e.g. []) could evaluate to False and lead to the wrong code block executed.

@steve-crouch
Copy link
Collaborator

Matt: Just need check the proposed solution still lines up.

@bielsnohr bielsnohr linked a pull request Apr 1, 2025 that will close this issue
@bielsnohr
Copy link
Collaborator

bielsnohr commented Apr 1, 2025

@p-j-smith thanks for this detailed issue and apologies it has taken us so long to get around to it! I've got a PR #451 up that should address everything. I have opted to put the stuff about "matching" in an optional exercise because I feel like it is just a bit of a stretch for everyone who might be taking the course, but could be really helpful for the more advanced learners. The point about using if expected_raises is not None was already incorporated into the course material at some other time, but regardless, thanks for pointing it out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants