Catch any warning on warns with no arg passed

[olgarithms]

Thanks for submitting a PR, your contribution is really appreciated!

Here is a quick checklist that should be present in PRs.

• Include documentation when adding new features.
• Include new tests or update existing tests when applicable.
• Allow maintainers to push and squash when merging my commits. Please uncheck this if you prefer to squash the commits yourself.

If this change fixes an issue, please:

• Closes Reducing user confusion from pytest.warns(None) #8645

Unless your change is trivial or a small documentation fix (e.g., a typo or reword of a small section) please:

• Create a new changelog file in the changelog folder, with a name like <ISSUE NUMBER>.<TYPE>.rst. See changelog/README.rst for details.

  Write sentences in the past or present tense, examples:

  • Improved verbose diff output with sequences.
  • Terminal summary statistics now use multiple colors.

  Also make sure to end the sentence with a ..

• Add yourself to AUTHORS in alphabetical order.

[olgarithms]

@Zac-HD this is the issue I worked on during the Mentored Sprints.

I had an issue where this test fails when I run the file directly, but passes when I run the entire test suite. Could you have a look?

Thank you :)

[olgarithms]

I had a closer look at the failing test and I see now what is happening.

warns(None) had indeed a weird behaviour: it was catching both ANY but also NO warnings. So, defaulting None to Warning would catch only warnings. The test would fail while exiting the context manager, if None behaved like Warning.

Instead, I kept the None behaviour for backwards-compatibility, but still added the DeprecationWarning, as well as changed the default from now on to be no arguments = Warning.

Let me know your thoughts and if this needs better documenting!

[RonnyPfannschmidt]

overall this looks nice - i do wonder about the case with None used in test_on_rm_rf_error

[RonnyPfannschmidt]

should this test use a mix of warns/catch_warnings to begin with?

imho there should be s single context manager which ensures no warning happens
but if it shows tricky to spell different quickly its fine to make that a follow-up task

[olgarithms]

That would be nice, but I am not sure if it is possible or how to do it.
As I understand we want to discourage folks from using None, but we are maintaining the functionality (and this test) only for backwards-compatibility. I would also be inclined to say that this test case could be removed(!), so maybe not super important to worry about it?

[Zac-HD]

should this test use a mix of warns/catch_warnings to begin with?

It's a weird test, but I think that Olga's edits are good - we'll remove/change it in 7.0 when the warns(None) deprecation is turned into an error.

imho there should be s single context manager which ensures no warning happens
but if it shows tricky to spell different quickly its fine to make that a follow-up task

IMO this is probably served with with warnings.catch_warnings(): (in the rare times that -Werror isn't enough); and if not it's a separate issue rather than aditional work on this PR.

[Zac-HD]

Thanks Olga! Just a few fiddly changes to the type annotations so that they reflect the non-deprecated behaviour, and adding a comment so that later readers know when our new test should be removed. Then we're done, and I'll be happy to merge!

[nicoddemus]

Thanks @olgarithms for the PR!

Left a small suggestion on the message. 👍

[Zac-HD]

Last thing, then I'm happy to merge.

[Zac-HD]

Suggested change

	expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]] = ...,
	expected_warning: Union[Type[Warning], Tuple[Type[Warning], ...]],

[olgarithms]

I had to add this again, as mypy was complaining that there is no overload of warns that allows for no arguments to be passed. The ... signifies the default argument that is passed in here.

[Zac-HD]

🎉

Congratulations and thanks again, Olga!

[tacaswell]

I am not sure if it is due to just this change or in collaboration with another change, but the suggested replacement is not equivalent as pytest.warns() will raise if nothing warns which breaks things like:

https://github.com/matplotlib/matplotlib/blob/c4dcf5e628d0441b21df1c8fe7cac4834a6944e9/lib/matplotlib/tests/test_mathtext.py#L440-L442

    with pytest.warns(None) as record:
        fig.canvas.draw()
    assert len(record) == 0, "\n".join(str(e.message) for e in record)

Which is probably a hold over from our migration from nose -> pytest a while back (and I'm about to open a PR to fix this and another case on the Matplotlib side), but it would be good if the deprecation message included a note about this difference (assuming it was intentional).

[Zac-HD]

Ah, I see - we should add some stdlib links to the relevant docs:

• If you want to get a list of warnings, but no error if no warnings were emitted, use warnings.catch_warnings()
• To say "any warning here is unexpected / an error", use with catch_warnings(): simplefilter("error"); ...

I didn't intend to break anything, but nor did I consider that people might be using pytest.warns() like warnings.catch_warnings(). My apologies for the inconvenience!

[nicoddemus]

Thanks @Zac-HD for the summary, I've created #9002 to track that.

[tacaswell]

https://xkcd.com/1172/ With enough users every bug is a feature and every bit of implementation detail in relied on by someone 🤷🏻‍♀️ . I run default branches of things to help catch these things early!

---

I use pytest almost every day (it is the 8th most common command line tool I run behind things like cd, ssh, ls, and git) and it is great 👍🏻 .