&benheid [BUG] allow alpha and coverage to be passed again via metrics to evaluate

[fkiraly]

This PR ensures pre-existing syntax to pass alpha and coverage via metrics to evaluate works again, fixing #5336.

Not commenting here on whether the status quo is a good idea or not (I think it was cleaner to remove it, or is, in the long run), but such a change should not happen without deprecation.

FYI @hazrulakmal.

Question to the reviewers, especially @hazrulakmal: do we need to change the naming convention to something else to ensure we do not change the names - or their order - again, accidentally?

Depends on #5337, so this change should trigger the test that is failing on main.

[benHeid]

The reindexing in line 290 (not changed) is the problem. Since the indexes are named differently. See pandas documentation: https://pandas.pydata.org/docs/reference/api/pandas.DataFrame.reindex.html

Thus, _get_column_order_and_datatype should return the same names as the DataFrame result have. Thus, it needs also to use the newly introduced for loop introduced in this PR.

This also requires some changes in the test, since accessing columns using df.col_name is not working anymore since the columns may be named as: test_PinballLoss_[0.1, 0.5, 0.9].

As an alternative, we could also try to rename all columns. However, this might lead to problems if multiple PinbalLosses are used in evaluate.

[fkiraly]

I see. So we need to restore the expected column names at least until the deprecation has taken place.

For the same reason I asked @hazrulakmal to keep the old column names for now (not breaking anything for the user!), I think we need to keep them here as well.

More precisely, my preferred solution would be:

• check whether in the current PR state, there are multiple columns relating to a loss.
• If not, replace it with the name that does not contain alpha or coverage

If this happens before the reindex, that should solve the issue?

[benHeid]

I see. So we need to restore the expected column names at least until the deprecation has taken place.

Yes. The challenge might be to ensure that we restore it correctly. I.e., we need to have a mapping somewhere from old to new.

If this happens before the reindex, that should solve the issue?

yes.

Would you then also directly introduce a legacy parameter that enables to control the naming of the columns?

[benHeid]

The fix is based on:

• remembering a mapping from new names to old ones
• introducing a flag to decide if old or new naming should be used!

I have not introduced a legacy parameter

[fkiraly]

amazing. Very elegant. I would have written sth more painful.

[fkiraly]

I approve your part, do but I cannot approve my own code...

[benHeid]

Looks good to me now. However, the code is partly written by me. However, I can technically approve it. If you are fine with my changes and I am fine with yours, it should be okay I suppose..

[fkiraly]

I'm fine with your changes, as said - that implies the four eyes principle is satisfied.