[ENH] rewrite test_probabilistic_metrics using proper pytest fixtures

[julia-kraus]

Reference Issues/PRs

Fixes #4907

What does this implement/fix? Explain your changes.

Moved the sample data generation inside fixtures, so it doesn't run with each import, but only when testing is performed.
Moreover refactored the tests such that the output tests for interval predictions and quantile predictions are separated.

Reason: With only one parametrized test were 96 parameter combinations, which made the test very slow and CPU heavy.
Also, some parameter combinations are not necessary (quantile metrics belong to quantile forecast only and interval metrics only to interval forecast) - please correct me if I'm wrong.
Separating the tests speeds up the testing process considerably and makes debugging easier.

Does your contribution introduce a new dependency? If yes, which one?

No

What should a reviewer concentrate their feedback on?

I left the original test_output with the many parameter combinations in working and refactored state commented out.
Decide if you still need it and if not, delete the commented out function.

Did you add any tests for the change?

Added the test test_sample_data() to check if the sample data generating fixture sample_data() is working correctly.

Any other comments?

The output test function contains many nested if - else statements. In my opinion, these should be separate tests with separate parametrized inputs, instead of putting all inputs in one test, and then distinguishing with if-else statements.

PR checklist

For all contributions

• I've added myself to the list of contributors with any new badges I've earned :-)
  How to: add yourself to the all-contributors file in the sktime root directory (not the CONTRIBUTORS.md). Common badges: code - fixing a bug, or adding code logic. doc - writing or improving documentation or docstrings. bug - reporting or diagnosing a bug (get this plus code if you also fixed the bug in the PR).maintenance - CI, test framework, release.
  See here for full badge reference
• Optionally, I've added myself and possibly others to the CODEOWNERS file - do this if you want to become the owner or maintainer of an estimator you added.
  See here for further details on the algorithm maintainer role.
• The PR title starts with either [ENH], [MNT], [DOC], or [BUG]. [BUG] - bugfix, [MNT] - CI, test framework, [ENH] - adding or improving code, [DOC] - writing or improving documentation or docstrings.

For new estimators

• I've added the estimator to the API reference - in docs/source/api_reference/taskname.rst, follow the pattern.
• I've added one or more illustrative usage examples to the docstring, in a pydocstyle compliant Examples section.
• If the estimator relies on a soft dependency, I've set the python_dependencies tag and ensured
  dependency isolation, see the estimator dependencies guide.

[fkiraly]

NICE! This speeds up test collection time massively for me.

Minor but important thing - linting is failing.

See here for linting & precommit:
https://www.sktime.net/en/latest/developer_guide/coding_standards.html

[fkiraly]

Also, some parameter combinations are not necessary (quantile metrics belong to quantile forecast only and interval metrics only to interval forecast) - please correct me if I'm wrong.

Hm, I think they should work at least the way round that quantile metrics should be applicable to quantile for interval forecasts - this makes mathematical sense since interval upper/lower define quantile forecasts. If this was tested before and didn't raise errors, it means it's actually implemente that way.

[julia-kraus]

ok, then I'd leave the original big function in the hope that the test server can handle it :)
thumbs up?

[fkiraly]

Thanks!

On details regarding what exactly to test, I think we ought to revisit this anyway with a class inheriting from TestAllObjects, so I'd not spend too much effort on the details of what is tested.

As far as I can see, your code is an 1:1 translation.

[julia-kraus]

@fkiraly
So I kept the two different functions for the tests for intervals and quantiles, but run all metrics on both.

The tests finish green on my machine. However, both take an hour to complete. This should be fine, but if you're looking for performance improvements in the future, that might be the place to look :)

[julia-kraus]

There's a memory error on the test runner as well.. I guess the vectorized tests from before (combining several data sets into one vector) were more memory efficient, however harder to read, debug and change. Is there a way that we ca slim down the test combinations?

[fkiraly]

There's a memory error on the test runner as well.. I guess the vectorized tests from before

This is unrelated to your changes - it is an instance of this sporadic error
#4610
which we haven't managed to properly diagnose yet.