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

MAINT: Replace use of pytest.warns(None) with warnings.catch_warnings #15192

Merged
merged 1 commit into from Dec 10, 2021

Conversation

WarrenWeckesser
Copy link
Member

The use of pytest.warns(None) is deprecated in pytest 7.0.0.
The alternative suggested by the deprecation warning, pytest.warns(),
does not match the old behavior. In fact, pytest.warns() is intended
specifically to raise an error if the code in the context does not raise
the given warning, and in pytest 7.0.0rc1, the default warning used by
pytest.warns() is the generic Warning class.

This issue is discussed in the following pytest issues:

There, the recommended alternative is to use warnings.catch_warnings.
That is the change made here.

Closes gh-15186

…ngs`.

The use of `pytest.warns(None)` is deprecated in pytest 7.0.0.
The alternative suggested by the deprecation warning, `pytest.warns()`,
does not match the old behavior.  In fact, `pytest.warns()` is intended
specifically to raise an error if the code in the context does not raise
the given warning, and in pytest 7.0.0rc1, the default warning used by
`pytest.warns()` is the generic `Warning` class.

This issue is discussed in the following pytest issues:

* pytest-dev/pytest#9002
* pytest-dev/pytest#9386

There, the recommended alternative is to use `warnings.catch_warnings`.
That is the change made here.

Closes scipygh-15186
@WarrenWeckesser WarrenWeckesser added the maintenance Items related to regular maintenance tasks label Dec 9, 2021
@WarrenWeckesser
Copy link
Member Author

WarrenWeckesser commented Dec 9, 2021

This fixes the last pytest-related failure in the Azure scipy.scipy (Main prerelease_deps_coverage_64bit_blas) check that I described in #15176 (comment).

There is a new failure in that check that is unrelated to this pull request. That failure is fixed by #15194.

@rlucas7
Copy link
Member

rlucas7 commented Dec 10, 2021

@WarrenWeckesser

Note: I was a little confused by one of the error message
but it looks like this is the default behavior for azure, according to this, azure passing a SIGINT and falls back to a SIGBREAK if SIGINT doesn't get the job to stop. That is what appears to be happening in one of the failures. The message to stdout is not indicative of what's going on but it appears to be a timeout issue. I'm not sure if that is something commonly happening but appears unrelated.

I looked at the docs for the warnings.catch_warnings here it mentioned this isn't threadsafe. I don't see threads being used directly in the two modules but I'm not sure about some of the repo level tooling, is that going to cause undefined behavior w/threads in these two tests?

everything else in this looks fine.

Copy link
Member

@rgommers rgommers left a comment

Choose a reason for hiding this comment

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

Thanks @WarrenWeckesser. Let me just merge this to get rid of the warning. I'll comment on the related issue.

@rgommers rgommers merged commit 2c77739 into scipy:master Dec 10, 2021
@rgommers rgommers added this to the 1.9.0 milestone Dec 10, 2021
@rgommers rgommers added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Dec 10, 2021
@WarrenWeckesser WarrenWeckesser deleted the replace-pytest-warns-none branch December 10, 2021 20:26
@WarrenWeckesser
Copy link
Member Author

@rlucas7 wrote

I looked at the docs for the warnings.catch_warnings here it mentioned this isn't threadsafe.

Thanks--yeah, I'm aware of that, it is an old problem. I don't know if the lack of thread safety for warnings.catch_warnings affects our tests. The reasons I'm OK with the change here is (1) we already use warning.catch_warnings elsewhere in the tests, and (2) behind the scenes, pytest.warns uses a class called WarningsRecorder that is a subclass of warning.catch_warnings, so apparently pytest.warns is not thread-safe either.

@tylerjereddy tylerjereddy modified the milestones: 1.9.0, 1.8.0 Dec 19, 2021
@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Dec 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Items related to regular maintenance tasks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fix use of pytest.warns(None) for pytest 7.0.0
4 participants