Galileo-Galilei / kedro-mlflow

A kedro-plugin for integration of mlflow capabilities inside kedro projects (especially machine learning model versioning and packaging)
https://kedro-mlflow.readthedocs.io/
Apache License 2.0
195 stars 29 forks source link

Missing project path prevents from running a project with kedro-airflow #54

Closed mwiewior closed 3 years ago

mwiewior commented 3 years ago

I propose to add project_path to run.py template in #53

        MlflowPipelineHook(
            model_name="{{ cookiecutter.python_package }}", conda_env="{{ cookiecutter.project_path }}/src/requirements.txt",
        ),
Galileo-Galilei commented 3 years ago

Hello,

I am aware of this problem and kedro-airflow specifies that paths need to be absolute. There is always a dilemma between keeping path relative (to make the project work "as is" when pulled) or making path absolute (for more declarative configuration and enhanced consistency).

I would incline to accept this, but I want to deprecate model_name and conda_env as arguments of MlflowPipelineHook in a future release because it is much more consistent to pass them as arguments for pipeline_ml rather than of a Hook and mainly because of #29. If we do this, this argument will no longer be auto-filled and you will have to specify this path manually when creating a PipelineML object so it will be up to you to decide whether you want a relative or absolute path.

We have to discuss it with @kaemo, but #29 is a pretty big change and it may not be completed until next release. We may merge your PR even if we expect it to be deprecated on the (short/medium/long?) run.

Let me know if it makes sense to you.

Galileo-Galilei commented 3 years ago

Juste for the record @mwiewior, I am curious for why this is actually needed:

Galileo-Galilei commented 3 years ago

I close the issue which is fixed in the upcoming release. Feel free to reopen if you have extra questions / informations.