Remove metric plots from metadata panel + remove pandas/plotly dependency from Kedro-viz

[rashidakanchwala]

Description

Resolves #999 and #1000

Development notes

I have removed the metrics plot and now we have a link that takes us straight to the plots on Experiment Tracking. Gabriel and Mackay are fine with the approach but they prefer if @AntonyMilneQB can also comment from a DS perspective.

QA notes

Checklist

• Read the contributing guidelines
• Opened this PR as a 'Draft Pull Request' if it is work-in-progress
• Updated the documentation to reflect the code changes
• Added new entries to the RELEASE.md file
• Added tests to cover my changes

[rashidakanchwala]

This is because kedro extras requirements are in the test_requirements and not in requirements. So e2e_tests keep failing

[antonymilne]

I don't think this is right (both before and after your change). The e2e test should be installing the pandas-iris starter project's requirements, not the kedro-viz requirements or test requirements. Look at the feature file:

        And I have run a non-interactive kedro new with pandas-iris starter
        And I have installed the project's requirements

The installation step is currently this:

@given("I have installed the project's requirements")
def install_project_requirements(context):
    """Run ``pip install -r src/requirements.txt``."""
    cmd = [context.pip, "install", "-r", str(context.requirements_path)]
    res = run(cmd, env=context.env)

    if res.returncode != OK_EXIT_CODE:
        print(res.stdout)
        print(res.stderr)
        assert False

The docstring is describing correctly what the function should do but actually the code isn't doing that. Instead it should be something like this:

@given("I have installed the project's requirements")
def install_project_requirements(context):
    """Run ``pip install -r src/requirements.txt``."""
     = run( [context.pip, "install", "-r", "src/requirements.txt"], env=context.env,  cwd=str(context.root_project_dir))

    if res.returncode != OK_EXIT_CODE:
        print(res.stdout)
        print(res.stderr)
        assert False

And then we can remove context.requirements_path completely.

[Huongg]

hey @rashidakanchwala quick check, is the known issue in your description now fixed? I thought the remaining issue would be the go back tab doesn't work in the flowchart only?

[rashidakanchwala]

hey @rashidakanchwala quick check, is the known issue in your description now fixed? I thought the remaining issue would be the go back tab doesn't work in the flowchart only?

You are right. thanks Huong <3

[Huongg]

LGTM, thanks @rashidakanchwala 😄

[antonymilne]

This is a great change, very happy to see it 👍 Generally looks good to me but two things we need to check.

Testing

See comments in the review but also very important to check: please try out on a project like pandas-iris that doesn't have any plotly or tracking datasets and do pip uninstall plotly. Does everything still work ok?

Where should links go from and to?

At the moment it seems we have this:

• tracking.metrics_dataset.MetricsDataSet ➡️ links to /experiment-tracking?view=Metrics. Fine but I wonder whether it would be possible some point in the future to point to the right graph exactly like #metric_name. It might also just make more sense to link to view=Overview instead (again, ideally to the right place using # at some point in the future). Not a biggy though.
• tracking.json_dataset.JSONDataSet ➡️ links to same place but I don't think that's intentional? It doesn't make much sense to link these to the metrics page, but it would make sense to link to view=Overview#... ultimately I think
• plots with versioned: True ➡️ no link to experiment tracking. Fine, but at some point in the future might be nice to put the link for these too

No need to implement all the above now, and I think it's fine how you have it as a quick win. But maybe as a separate ticket it would be good to think about exactly what should link where.

[antonymilne]

I don't think this is right (both before and after your change). The e2e test should be installing the pandas-iris starter project's requirements, not the kedro-viz requirements or test requirements. Look at the feature file:

        And I have run a non-interactive kedro new with pandas-iris starter
        And I have installed the project's requirements

The installation step is currently this:

@given("I have installed the project's requirements")
def install_project_requirements(context):
    """Run ``pip install -r src/requirements.txt``."""
    cmd = [context.pip, "install", "-r", str(context.requirements_path)]
    res = run(cmd, env=context.env)

    if res.returncode != OK_EXIT_CODE:
        print(res.stdout)
        print(res.stderr)
        assert False

The docstring is describing correctly what the function should do but actually the code isn't doing that. Instead it should be something like this:

@given("I have installed the project's requirements")
def install_project_requirements(context):
    """Run ``pip install -r src/requirements.txt``."""
     = run( [context.pip, "install", "-r", "src/requirements.txt"], env=context.env,  cwd=str(context.root_project_dir))

    if res.returncode != OK_EXIT_CODE:
        print(res.stdout)
        print(res.stderr)
        assert False

And then we can remove context.requirements_path completely.

[antonymilne]

Do we really need this change? See above comment on e2e tests. I think we can probably do without the additional datasets here.

[rashidakanchwala]

we need it for unit tests.

[rashidakanchwala]

This is a great change, very happy to see it 👍 Generally looks good to me but two things we need to check.

Testing

See comments in the review but also very important to check: please try out on a project like pandas-iris that doesn't have any plotly or tracking datasets and do pip uninstall plotly. Does everything still work ok?

Where should links go from and to?

At the moment it seems we have this:

• tracking.metrics_dataset.MetricsDataSet ➡️ links to /experiment-tracking?view=Metrics. Fine but I wonder whether it would be possible some point in the future to point to the right graph exactly like #metric_name. It might also just make more sense to link to view=Overview instead (again, ideally to the right place using # at some point in the future). Not a biggy though.
• tracking.json_dataset.JSONDataSet ➡️ links to same place but I don't think that's intentional? It doesn't make much sense to link these to the metrics page, but it would make sense to link to view=Overview#... ultimately I think
• plots with versioned: True ➡️ no link to experiment tracking. Fine, but at some point in the future might be nice to put the link for these too

No need to implement all the above now, and I think it's fine how you have it as a quick win. But maybe as a separate ticket it would be good to think about exactly what should link where.

Thanks Antony. the JSONDataSet should no longer have any links but I think your idea to link it to Overview tab is better so I will do that.

[antonymilne]

Looks great! ⭐

Just to check, did you try this:

very important to check: please try out on a project like pandas-iris that doesn't have any plotly or tracking datasets and do pip uninstall plotly. Does everything still work ok?

Also what do you think about adding anchors to the experiment tracking page so we can link to the exact right part? If you think it's a good idea then please do make a new issue for it 🙂 But it's not very important.

[rashidakanchwala]

Looks great! ⭐

Just to check, did you try this:

very important to check: please try out on a project like pandas-iris that doesn't have any plotly or tracking datasets and do pip uninstall plotly. Does everything still work ok?

Also what do you think about adding anchors to the experiment tracking page so we can link to the exact right part? If you think it's a good idea then please do make a new issue for it 🙂 But it's not very important.

Yes, it works with pandas-iris :)

The adding anchors is a known issue and @Huongg said it's difficult to do now but she will be working on it.