[BUG] Fix ARCH._check_predict_proba #5349

[Vasudeva-bit]

Reference Issues/PRs

Fixes #5349

What does this implement/fix? Explain your changes.

Changes a if (check) condition in _check_predict_proba function of sktime/forecasting/tests/test_all_forecasters.py to preserve column name in univariate forecaster predictions.

Upon fixing this issue, ARCH doesn't require function _predict_proba as BaseForecaster's _predict_proba is sufficient. Hence, removed that function.

What should a reviewer concentrate their feedback on?

A few tests are expecting name to be preserved even if it is None, here. Thats why I didn't handle the None case separately. If the expected column name is 0 when original name is None, below code may be appropriate as recommended by @yarnabrina.

if isinstance(y_train, pd.Series) and y_train.name:
    assert (pred_cols == y_train.name).all()
elif isinstance(y_train, pd.Series):  # automatically implies empty name
    assert (pred_cols == pd.Index([0])).all()
else:
    assert (pred_cols == y_train.columns).all()

Additional context

Any changes in this test will affect existing univariate probabilistic forecasters only such as NaiveForecaster.

PR checklist

For all contributions

• 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.

[fkiraly]

I've updated this PR with code along the lines suggested. I will turn on tests for all forecasters so we can see what happens.

[fkiraly]

Ok, I'm confused now - nothing else seems to fail. Any explanations?

[Vasudeva-bit]

After debugging for a while,

1. Firstly, the function _check_predict_proba is only testing for y_train generated from _make_data(n_columns=n_columns). If the forecaster is univariate, then the y_train generated would always have column name None of type str. The _check_predict_proba checks whether the _predict_proba is changing None to 0 internally. Since every forecaster's _predict_proba doing it, all forecasters passed.
2. Actually ARCH should fail this test, because _predict_proba function is deleted in this PR. But, the type(y_train.name) is str ('None'), not None. Therefore, None case is skipped and checks whether (pred_cols == y_train.name).all() which is None of str equals None of str so, ARCH also passed.

I think it's my bad, there is no bug in _check_predict_proba as it is not designed to check whether column names are preserved. Changes should only be in ARCH's _predict_proba like below:

1. If the column name is None of type None/str, do pred_dist.name = pd.Index([0]).
2. Otherwise, just return output of Super()._predict_proba() as it is, i.e., column names are preserved by default.

Hope it makes some sense now.

[fkiraly]

I see, so the issue was after all isolated to uarch - makes sense. Lets merge this as a fix then.