Error on xarray warnings in tests?

[max-sixty]

What is your issue?

We've done a superb job of cutting the number of warnings in #3266.

On another project I've been spending time with recently, we raise an error on any warnings in the test suite. It's easy mode — the dependencies are locked (it's not python...), but I wonder whether we can do something some of the way with this:

Would it be worth failing on:

• Warnings from within xarray
  • There's no chance of an external change causing main to fail. When we deprecate something, we'd update calling code with it.
  • This would also ensure doctests & docs don't use old versions. Currently doctests have some warnings.
• Warnings from the min-versions test
  • It prevents us from using outdated APIs
  • min-versions are fixed dependencies, so also no chance of an external change causing main to fail
  • It would fail in a more deliberate way than the upstream tests do now
  • OTOH, possibly it would discourage us from bumping those min versions — the burden falls on the bumper — already a generous PR!
  • ...and it's not perfectly matched — really we want to update from an old API before it changes in the new version, not before it becomes deprecated in an old version

[headtr1ck]

There is still the problem that we support several outdated backends that will raise warnings, like pydap the outdated Version thingy. This will also happen in the min-deps builds.

[max-sixty]

There is still the problem that we support several outdated backends that will raise warnings, like pydap the outdated Version thingy. This will also happen in the min-deps builds.

Yes, good point, and I should have added above — it's still possible to ignore warnings at the test site explicitly (@pytest.mark.filterwarnings("ignore:foo"). So it's good, but also a chore, to mark each instance of those...

[mathause]

I am a bit skeptical - we test a large range of versions and the kinds of warnings we get can be quite different. I fear that we get too many false positives and end up wrapping everything with filterwarnings("ignore").

[max-sixty]

I am a bit skeptical - we test a large range of versions and the kinds of warnings we get can be quite different. I fear that we get too many false positives and end up wrapping everything with filterwarnings("ignore").

Yes, very fair.

Though to confirm — this is a concern for the "Warnings from the min-versions test", rather than warnings that xarray issues? Or for both?

[mathause]

Yes I mean the external ones. I agree that it is a good idea to disallow internal warnings (sometimes PRs introduce them without realizing...). And of course I also agree that we should act when then deprecation warnings are being thrown and not when the stuff is actually removed.

[max-sixty]

We now disallow internal warnings, with a list of exceptions, so closing as completed