Objective function type annotation relaxation to `BaseTrial` from `Trial`

[hvy]

Motivation

The objective function type hint says that its parameter should be of type Trial. However, https://github.com/optuna/optuna/pull/1503 will allow FrozenTrials (which will derive from BaseTrial but not Trial) to be passed to the objective function. Type hints should reflect this change.

More precisely, the type annotation of ObjectiveFuncType should be updated and all definitions of objective functions in e.g. tests or examples.

~It's open for discussion given the fact that user objective function type hints will break.~ (Edited 2020-08-19)

[HideakiImamura]

Thanks for raising an important issue. I agree with the idea of changing the type annotation of the objective function. The changes for type annotations in optuna/optuna directory may affect the user codes, but those in optuna/tests or optuna/example directories don't?

Those tasks to change annotations in tests or example directories seem obvious, so how about labeling them contribution-welcome?

[hvy]

The changes for type annotations in optuna/optuna directory may affect the user codes, but those in optuna/tests or optuna/example directories don't?

Correct.

Those tasks to change annotations in tests or example directories seem obvious, so how about labeling them contribution-welcome?

Sure, but I think those changes should derive from updating ObjectiveFuncType. Maybe it's sensible to make this change now, in the same release as #1503, i.e. v2.1, labeling this as contribution-welcome at the same time. Unfortunately, as far as I know, there's no good practice for going through a "deprecation" phase for type hints (Union[Trial, BaseTrial] will still break existing type hints).

[github-actions[bot]]

This issue has not seen any recent activity.

[hvy]

Several developers seem to agree that it's okay to proceed with this change given the reasons above. Let me label this ticket as contribution-welcome.

[github-actions[bot]]

This issue has not seen any recent activity.

[pchelaa]

Hi! I would like to work on that issue.

It seems to me that more changes to optuna/optuna are necessary. The problems is with callback pruners that receive Trial object. Since they are called from objective function which now should accept BaseTrial it will break typing. Also simply switching to BaseTrial in pruners will not work because some of them use trial's study object which is not included in BaseTrial, for example LightGBMPruningCallback. So some changes to pruners are required. Am I right?

[hvy]

I think you're right. The pruning callbacks should work with FrozenTrial (do nothing/do not prune) as well so we might have to update the logic of some (e.g. make sure it doesn't raise an error trying to access study). Since each integration pruning callback differs quite a bit in how they're implemented and used, we probably have to to through them one by one.

[HideakiImamura]

This is still an open and very important.

[github-actions[bot]]

This issue has not seen any recent activity.

[github-actions[bot]]

This issue was closed automatically because it had not seen any recent activity. If you want to discuss it, you can reopen it freely.