Apply deprecation decorator. #1413
Conversation
toshihikoyanase
added
the
code-fix
There was a problem hiding this comment.
Thanks for the follow up. I left some nits.
A question also, should we apply it to best_booster https://46573-122299416-gh.circle-artifacts.com/0/docs/build/html/reference/integration.html#optuna.integration.lightgbm.LightGBMTuner.best_booster ? It now hit me that we haven't tested this decorator on custom getters/setters.
| @@ -42,3 +41,9 @@ def test_study_summary_deprecated(): | |||
| datetime_start=datetime.datetime.now(), | |||
| study_id=0, | |||
| ) | |||
|
|
|||
|
|
|||
| def test_trial_pruned_deprecated() -> None: | |||
Co-authored-by: Hiroyuki Vincent Yamazaki <hiroyuki.vincent.yamazaki@gmail.com>
|
@toshihikoyanase I have another comment. How about applying the deprecation decorator to |
|
@hvy @HideakiImamura Thank you for your comment on the
|
|
Mypy does not support the type checking of decorated properties.
As suggested in python/mypy#1362, I add |
| "Please get the best booster via " | ||
| ":class:`~optuna.integration.lightgbm.LightGBMTuner.get_best_booster` instead." | ||
| ), | ||
| ) |
There was a problem hiding this comment.
Mostly wondering but does having the deprecated also work?
There was a problem hiding this comment.
I'm sorry but I couldn't get the point. If you mean the confirmation of the behavior of best_booster property with @decorated, it works as expected :
# examples/lightgbm_tuner_simple.py
....
# model = lgb.train(
# params, dtrain, valid_sets=[dtrain, dval], verbose_eval=100, early_stopping_rounds=100,
# )
# model = lgb.train(
# params, dtrain, valid_sets=[dtrain, dval], verbose_eval=100, early_stopping_rounds=100,
# )
lgbt = lgb.LightGBMTuner(
params, dtrain, valid_sets=[dtrain, dval], verbose_eval=100, early_stopping_rounds=100,
)
lgbt.run()
model = lgbt.best_boosteroptuna/_deprecated.py:110: DeprecationWarning: best_booster has been deprecated in v1.4.0. This feature will be removed in v3.0.0.
DeprecationWarning,There was a problem hiding this comment.
Sorry, I meant having the deprecated decorator outermost (applied after property).
Motivation
Followup #1382.
Description of the changes
This PR applies the deprecation decorator to the following classes:
optuna.integratoin.CmaEsSampleroptuna.struct.FrozenTrialoptuna.struct.StudySummaryoptuna.struct.TrialPruned