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.

Sign up for GitHub

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

xfail for test_G_star_Row_Standardized #336

Merged
merged 4 commits into from
Jul 21, 2024
Merged

xfail for test_G_star_Row_Standardized #336

Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
50f12ac
resolve #331
jGaboardi Jul 20, 2024
704f3f7
more granular xfail
jGaboardi Jul 20, 2024
6bbf8e4
Merge remote-tracking branch 'upstream/main' into GH331_xfail_test_G_…
jGaboardi Jul 20, 2024
91df530
split into 2 tests -- warning & values
jGaboardi Jul 20, 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
7 changes: 6 additions & 1 deletion esda/tests/test_getisord.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,14 @@ def test_G_star_Local_Binary(self, w):
np.testing.assert_allclose(lg.p_sim[0], 0.102, rtol=RTOL, atol=ATOL)

@parametrize_w
@pytest.mark.xfail(
reason="Intermittently does not warn for W param; reason unknown; see gh#331"
)
def test_G_star_Row_Standardized(self, w):
with pytest.warns(UserWarning, match="Gi\\* requested, but"):
with pytest.warns(UserWarning, match="Gi\\* requested, but") as record:
lg = getisord.G_Local(self.y, w, transform="R", star=True, seed=10)
if not record:
pytest.fail("Expected a warning!")
Copy link
Member

Choose a reason for hiding this comment

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

Why do you do this?

Copy link
Member Author

Choose a reason for hiding this comment

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

As a fail safe so it fails exactly when no warnings are emitted. I could not figure out how to add that granularity to parametrize_w without it affecting other tests. Maybe I'm missing something easy? Any thoughts?

I couldn't determine the proper raises argument for pytest.mark.xfail and I think having only a general failure is too vague.

Copy link
Member

Choose a reason for hiding this comment

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

But this makes no difference, no? You are just doing explicitly what pytest would do here anyway. I probably don't follow here...

We can also split this into two tests, one checking the warning and the other the values.

Copy link
Member Author

Choose a reason for hiding this comment

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

I probably don't follow here...

I didn't explain well. What I mean it that I could not find the proper pytest error type to include in the xfail(raise=) parameter - the error raised when no warnings are emitted when in pytest.warns context.

We can also split this into two tests, one checking the warning and the other the values.

This may be the way to go if there isn't a more elegant solution that I am overlooking.

Copy link
Member

Choose a reason for hiding this comment

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

I'd probably split it.

np.testing.assert_allclose(lg.Zs[0], -0.62488094, rtol=RTOL, atol=ATOL)
np.testing.assert_allclose(lg.p_sim[0], 0.102, rtol=RTOL, atol=ATOL)

Expand Down