Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Avoid installing the latest MLfow to prevent doctests from failing #3135

Merged
merged 2 commits into from Nov 30, 2021

Conversation

himkt
Copy link
Member

@himkt himkt commented Nov 30, 2021

Motivation

Although I haven't investigated the cause yet, the doctest was failing with the latest MLflow.
https://github.com/optuna/optuna/runs/4368455449?check_suite_focus=true

I confirmed the problem doesn't occur with MLflow v1.21.0.
https://github.com/optuna/optuna/runs/4368582442?check_suite_focus=true
So, as a tentative patch, I introduce the version constraint.

Description of the changes

@himkt himkt changed the title [WIP] Avoid doctests failing in MLFlow Avoid installing the latest MLfow to prevent doctests from failing Nov 30, 2021
@himkt
Copy link
Member Author

himkt commented Nov 30, 2021

INFO: 'my_study' does not exist. Creating a new experiment

We expect callback to log a message by the above format.
MLflow changes a way of logging from print to logging from v1.22.0.
https://github.com/mlflow/mlflow/pull/5012/files#diff-b33420360cc29a5224c8d3080bbfcaa0ac1c8891416137e13be044fcdfdd26f9L84-R113

ref. mlflow/mlflow#5012 pointed out by @nzw0301

We have to follow the change in MLflow.

@himkt
Copy link
Member Author

himkt commented Nov 30, 2021

We'll be able to update our doctest to follow the change in MLflow.
I'll look at the change and fix it later but I'd merge this PR for the first aid.
If someone is interested in, please comment in #3136 and work on! I'll be available after tomorrow. 🐱

@himkt himkt requested review from hvy and nzw0301 November 30, 2021 14:39
@himkt
Copy link
Member Author

himkt commented Nov 30, 2021

Sorry for the by-name mention but please take a look @nzw0301 @hvy.
I believe we share the context of this problem.

Copy link
Member

@hvy hvy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@hvy hvy self-assigned this Nov 30, 2021
@hvy hvy added this to the v3.0.0-a1 milestone Nov 30, 2021
Copy link
Member

@nzw0301 nzw0301 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. thanks!

@nzw0301 nzw0301 merged commit 2a9f863 into optuna:master Nov 30, 2021
@himkt himkt deleted the patch/mlflow branch November 30, 2021 14:54
@himkt himkt added the CI Continuous integration. label Dec 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CI Continuous integration.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants